Jump to page: 1 2
Thread overview
"Good PR" mechanical check
Jan 12, 2016
Martin Drašar
Jan 13, 2016
H. S. Teoh
Jan 13, 2016
Brian Schott
Jan 12, 2016
Rikki Cattermole
Jan 12, 2016
Adam D. Ruppe
Jan 12, 2016
w0rp
Jan 12, 2016
Walter Bright
Jan 12, 2016
John Colvin
Jan 12, 2016
Jacob Carlborg
Jan 12, 2016
Sebastiaan Koppe
Jan 12, 2016
tsbockman
Jan 12, 2016
rsw0x
Jan 12, 2016
Mathias Lang
January 12, 2016
Related to https://github.com/D-Programming-Language/dlang.org/pull/1191:

A friend who is in the GNU community told me a while ago they have a mechanical style checker that people can run against their proposed patches to make sure the patches have a style consistent with the one enforced by the GNU style. I forgot the name, something like "good-patch". I'll shoot him an email.

Similarly, I think it would help us to release a tool in the tools/ repo that analyzes a would-be Phobos pull request and ensures it's styled the same way as most of Phobos: braces on their own lines, whitespace inserted a specific way, no hard tabs, etc. etc. Then people can run the tool before even submitting a PR to make sure there's no problem of that kind.

Over the years I've developed some adaptability to style, and I can do Phobos' style no problem even though it wouldn't be my first preference (I favor Egyptian braces). But seeing a patchwork of styles within the same project, same file, and sometimes even the same pull request is quite jarring at least to me.

Who would like to embark on writing such a tool?


Thanks,

Andrei
January 12, 2016
Dne 12.1.2016 v 14:34 Andrei Alexandrescu via Digitalmars-d napsal(a):
> Related to https://github.com/D-Programming-Language/dlang.org/pull/1191:
> 
> A friend who is in the GNU community told me a while ago they have a mechanical style checker that people can run against their proposed patches to make sure the patches have a style consistent with the one enforced by the GNU style. I forgot the name, something like "good-patch". I'll shoot him an email.
> 
> Similarly, I think it would help us to release a tool in the tools/ repo that analyzes a would-be Phobos pull request and ensures it's styled the same way as most of Phobos: braces on their own lines, whitespace inserted a specific way, no hard tabs, etc. etc. Then people can run the tool before even submitting a PR to make sure there's no problem of that kind.
> 
> Over the years I've developed some adaptability to style, and I can do Phobos' style no problem even though it wouldn't be my first preference (I favor Egyptian braces). But seeing a patchwork of styles within the same project, same file, and sometimes even the same pull request is quite jarring at least to me.
> 
> Who would like to embark on writing such a tool?
> 
> 
> Thanks,
> 
> Andrei

Wouldn't it be sufficient to mandate usage of dfmt with proper settings before submitting a PR?

Martin



January 13, 2016
On 13/01/16 2:34 AM, Andrei Alexandrescu wrote:
> Related to https://github.com/D-Programming-Language/dlang.org/pull/1191:
>
> A friend who is in the GNU community told me a while ago they have a
> mechanical style checker that people can run against their proposed
> patches to make sure the patches have a style consistent with the one
> enforced by the GNU style. I forgot the name, something like
> "good-patch". I'll shoot him an email.
>
> Similarly, I think it would help us to release a tool in the tools/ repo
> that analyzes a would-be Phobos pull request and ensures it's styled the
> same way as most of Phobos: braces on their own lines, whitespace
> inserted a specific way, no hard tabs, etc. etc. Then people can run the
> tool before even submitting a PR to make sure there's no problem of that
> kind.
>
> Over the years I've developed some adaptability to style, and I can do
> Phobos' style no problem even though it wouldn't be my first preference
> (I favor Egyptian braces). But seeing a patchwork of styles within the
> same project, same file, and sometimes even the same pull request is
> quite jarring at least to me.
>
> Who would like to embark on writing such a tool?
>
>
> Thanks,
>
> Andrei

dfmt + create patch compared to before and after format.
No need for a whole new tool.
January 12, 2016
On Tuesday, 12 January 2016 at 13:34:25 UTC, Andrei Alexandrescu wrote:
> Similarly, I think it would help us to release a tool in the tools/ repo that analyzes a would-be Phobos pull request and ensures it's styled the same way as most of Phobos


I'm not sure if git supports this but I think it should be done fully automatically. Not even something the user runs, just when they open the pull request, it reformats the code.

I'm pretty sure dfmt is up to the task in 99% of cases already.
January 12, 2016
I think using dfmt for this is a good idea. If there any problems with dfmt which would prevent it from being used on Phobos, the problems can be patched and then that would strengthen dfmt.
January 12, 2016
On 1/12/2016 6:53 AM, Adam D. Ruppe wrote:
> I'm pretty sure dfmt is up to the task in 99% of cases already.

The last 1% always takes 99% of the dev time :-(
January 12, 2016
On Tuesday, 12 January 2016 at 13:34:25 UTC, Andrei Alexandrescu wrote:
> [...]

I realize that dfmt may need some upgrades first, but isn't it about time to just suck it up and dfmt the whole of phobos and druntime?

It will mess with the "git blame", true - but it will do so *once* and end tedious manual inspection of formatting for pull requests forever.
January 12, 2016
On Tuesday, 12 January 2016 at 18:25:48 UTC, tsbockman wrote:
> On Tuesday, 12 January 2016 at 13:34:25 UTC, Andrei Alexandrescu wrote:
>> [...]
>
> I realize that dfmt may need some upgrades first, but isn't it about time to just suck it up and dfmt the whole of phobos and druntime?
>
> It will mess with the "git blame", true - but it will do so *once* and end tedious manual inspection of formatting for pull requests forever.

Doubt I'm alone in that I'd be more willing to contribute if phobos and druntime used dfmt. I personally use a style near the complete opposite of phobos/druntime (2 spaces per indent, OTBS etc.) It's a chore to have to scrutinize every line to make sure it matches the style guide instead of just running it through a tool.
January 12, 2016
On Tuesday, 12 January 2016 at 17:22:16 UTC, Walter Bright wrote:
> On 1/12/2016 6:53 AM, Adam D. Ruppe wrote:
>> I'm pretty sure dfmt is up to the task in 99% of cases already.
>
> The last 1% always takes 99% of the dev time :-(

But in this case, the 1% doesn't actually have to be fixed (although of course, the smaller the better), it's just the 1% of the work left to be done manually, where currently we do 100% manually.
January 12, 2016
On 01/12/2016 01:25 PM, tsbockman wrote:
> On Tuesday, 12 January 2016 at 13:34:25 UTC, Andrei Alexandrescu wrote:
>> [...]
>
> I realize that dfmt may need some upgrades first, but isn't it about
> time to just suck it up and dfmt the whole of phobos and druntime?
>
> It will mess with the "git blame", true - but it will do so *once* and
> end tedious manual inspection of formatting for pull requests forever.

I would support that. We need a strong champion there. Brian wanted to get to it for a long time, but didn't muster the time. -- Andrei
« First   ‹ Prev
1 2