| Thread overview | |||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
January 12, 2016 "Good PR" mechanical check | ||||
|---|---|---|---|---|
| ||||
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 Re: "Good PR" mechanical check | ||||
|---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu Attachments:
| 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 Re: "Good PR" mechanical check | ||||
|---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | 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 Re: "Good PR" mechanical check | ||||
|---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | 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 Re: "Good PR" mechanical check | ||||
|---|---|---|---|---|
| ||||
Posted in reply to Adam D. Ruppe | 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 Re: "Good PR" mechanical check | ||||
|---|---|---|---|---|
| ||||
Posted in reply to Adam D. Ruppe | 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 Re: "Good PR" mechanical check | ||||
|---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | 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 Re: "Good PR" mechanical check | ||||
|---|---|---|---|---|
| ||||
Posted in reply to tsbockman | 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 Re: "Good PR" mechanical check | ||||
|---|---|---|---|---|
| ||||
Posted in reply to Walter Bright | 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 Re: "Good PR" mechanical check | ||||
|---|---|---|---|---|
| ||||
Posted in reply to tsbockman | 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
| |||
Copyright © 1999-2021 by the D Language Foundation
Permalink
Reply