digitalmars.D - Automatically enforce Phobos's styleguide
- Seb (38/40) Apr 26 2016 Hey all,
- tsbockman (7/9) Apr 26 2016 It's not so much a matter of "leaks" - rather, much of DMD,
- Mark Isaacson (7/7) Apr 26 2016 I maintain some of the linters at work. One suggestion I'd have
- Seb (11/23) Apr 26 2016 Great news:
- Andrei Alexandrescu (4/28) Apr 27 2016 Nice initiative. Thanks for doing this. Please stay on 4243 and merge it...
- Seb (12/30) Apr 27 2016 Just a quick update that #4243 is in and the linting bot is now
- Andrei Alexandrescu (2/8) Apr 28 2016 Do you have an example of an error flow? -- Andrei
- Seb (6/18) Apr 28 2016 I accidentally proved that it does check for errors:
- Andrei Alexandrescu (3/17) Apr 28 2016 Hmmm... not to clear error messages - something we could improve in the
Hey all,
tl;dr: I want to pull improve Phobos code style for the greater
good of having a bot that automatically checks the coding style.
This should be quite useful for submitters. I hope we all agree
that the time spent "nitpicking" is wasted, we overlook much (see
below) and we can do better!
Agenda
------
The idea is quite simple:
1) Fix one violation for the entire codebase
2) Enable it in the bot.
3) Pick next violation and goto 1)
First violation: space between operators
----------------------------------------
Can be easily fixed by running this:
```
sed -i "s/switch(/switch (/" **/*.d
sed -i "s/foreach(/foreach (/" **/*.d
sed -i "s/foreach_reverse(/foreach_reverse (/" **/*.d
sed -i "s/while(/while (/" **/*.d
sed -i "s/if(/if (/" **/*.d
```
I started with some PRs (one is way to large to review):
https://github.com/dlang/phobos/pull/4239
https://github.com/dlang/phobos/pull/4240
https://github.com/dlang/phobos/pull/4241
https://github.com/dlang/phobos/pull/4242
How big is this change?
-----------------------
grep -E "(for|foreach|foreach_reverse|if|while)\(" $(find .
-name '*.d') | wc -l
3505
As expected a styleguide without a automatically enforcing it,
leaks ...
Disadvantages
-------------
This potentially might create some troubles with forks or pending
PRs.
-> If anyone objects the introduction of automatic linting to
Phobos, please raise your voice!
Apr 26 2016
tsbockman <thomas.bockman gmail.com> On Tuesday, 26 April 2016 at 19:04:44 UTC, Seb wrote:As expected a styleguide without a automatically enforcing it, leaks ...It's not so much a matter of "leaks" - rather, much of DMD, D-runtime, and Phobos pre-date the style rules, and so never attempted to follow them in the first place. Indeed, some of the rules we have now were chosen simply by surveying the existing eclectic code base to determine which conventions were *most common* - in some cases by slim margins...
Apr 26 2016
I maintain some of the linters at work. One suggestion I'd have (perhaps you're already on it) is that a linter that can both detect and prompt to auto-fix the issues is at least an order of magnitude more useful. People fix things more, people like the linters more, it avoids the fallacy of trying to get people to read big paragraphs of how to fix things themselves... it's just generally a much more awesome experience. Would recommend.
Apr 26 2016
On Tuesday, 26 April 2016 at 19:04:44 UTC, Seb wrote:Hey all, tl;dr: I want to pull improve Phobos code style for the greater good of having a bot that automatically checks the coding style. This should be quite useful for submitters. I hope we all agree that the time spent "nitpicking" is wasted, we overlook much (see below) and we can do better! Agenda ------ The idea is quite simple: 1) Fix one violation for the entire codebase 2) Enable it in the bot. 3) Pick next violation and goto 1)Great news: 2) The Travis bot passes :) As mentioned I decreased the linting to a minimum, but now we do have it :) Future work can fix more violations in the Phobos codebase - small low hanging fruits include e.g. > 120 lines or undocumented public methods. For more details: https://github.com/dlang/phobos/pull/4243
Apr 26 2016
On 04/26/2016 09:36 PM, Seb wrote:On Tuesday, 26 April 2016 at 19:04:44 UTC, Seb wrote:Nice initiative. Thanks for doing this. Please stay on 4243 and merge it as soon as possible, so we don't have ripples following the style change from pulling other diffs. Thx! -- AndreiHey all, tl;dr: I want to pull improve Phobos code style for the greater good of having a bot that automatically checks the coding style. This should be quite useful for submitters. I hope we all agree that the time spent "nitpicking" is wasted, we overlook much (see below) and we can do better! Agenda ------ The idea is quite simple: 1) Fix one violation for the entire codebase 2) Enable it in the bot. 3) Pick next violation and goto 1)Great news: 2) The Travis bot passes :) As mentioned I decreased the linting to a minimum, but now we do have it :) Future work can fix more violations in the Phobos codebase - small low hanging fruits include e.g. > 120 lines or undocumented public methods. For more details: https://github.com/dlang/phobos/pull/4243
Apr 27 2016
On Wednesday, 27 April 2016 at 12:38:55 UTC, Andrei Alexandrescu wrote:On 04/26/2016 09:36 PM, Seb wrote:running. As I mentioned earlier there are still many trivial flags of Dscanner's static analysis that could be enabled. I marked them with "FIXME", so anyone feel free to have a look. Moreover (once dub is 1.0 and released together with dmd), we want to enable the same linting in the Makefile. Mark Isaacson: Great idea, but that sounds more like a plugin for an IDE. dfmt usually fixes most of the style issues, however Dscanner does also analysis of the code.Great news: 2) The Travis bot passes :) As mentioned I decreased the linting to a minimum, but now we do have it :) Future work can fix more violations in the Phobos codebase - small low hanging fruits include e.g. > 120 lines or undocumented public methods. For more details: https://github.com/dlang/phobos/pull/4243Nice initiative. Thanks for doing this. Please stay on 4243 and merge it as soon as possible, so we don't have ripples following the style change from pulling other diffs. Thx! -- Andrei
Apr 27 2016
On 04/27/2016 11:41 PM, Seb wrote:As I mentioned earlier there are still many trivial flags of Dscanner's static analysis that could be enabled. I marked them with "FIXME", so anyone feel free to have a look. Moreover (once dub is 1.0 and released together with dmd), we want to enable the same linting in the Makefile.Do you have an example of an error flow? -- Andrei
Apr 28 2016
On Thursday, 28 April 2016 at 13:46:37 UTC, Andrei Alexandrescu wrote:On 04/27/2016 11:41 PM, Seb wrote:I accidentally proved that it does check for errors: https://github.com/dlang/phobos/pull/4255 You can see the history here: https://travis-ci.org/dlang/phobos/buildsnow running. As I mentioned earlier there are still many trivial flags of Dscanner's static analysis that could be enabled. I marked them with "FIXME", so anyone feel free to have a look. Moreover (once dub is 1.0 and released together with dmd), we want to enable the same linting in the Makefile.Do you have an example of an error flow? -- Andrei
Apr 28 2016
On 04/28/2016 09:50 AM, Seb wrote:On Thursday, 28 April 2016 at 13:46:37 UTC, Andrei Alexandrescu wrote:Hmmm... not to clear error messages - something we could improve in the near future. -- AndreiOn 04/27/2016 11:41 PM, Seb wrote:I accidentally proved that it does check for errors: https://github.com/dlang/phobos/pull/4255 You can see the history here: https://travis-ci.org/dlang/phobos/buildsAs I mentioned earlier there are still many trivial flags of Dscanner's static analysis that could be enabled. I marked them with "FIXME", so anyone feel free to have a look. Moreover (once dub is 1.0 and released together with dmd), we want to enable the same linting in the Makefile.Do you have an example of an error flow? -- Andrei
Apr 28 2016