Coding Style

Kamil Paral kparal at redhat.com
Tue Jun 16 10:25:22 UTC 2015


> After the meeting earlier today, I wanted to make a slightly different
> proposal.
> 
>  - if flake8 config in arcanist can be so configured, set max line
>    width to 120 instead of 79

On my 1920px monitor, two windows with 100 columns fit perfectly next to each other (11px font). Is there a strong desire to go over 100 cols? It would make file comparisons less practical. And 100 (well, 99) is still PEP8-approved.

I have no problems exceeding that limit in certain specific cases. For example, because we often bundle documentation with the source code, and we format it with rst, it might happen that a large block of text is indented far right and it makes it very long. Or there are some code samples which can be wrapped but it makes them hard to read. In such cases I have no problem having these particular lines go to 120 (or even more, if needed). But this would be very exceptional and almost never applied to regular code - no need to worry about linter exceptions discussions, I think.

PS: It seems that flake8 can't be configured in arcanist at the moment. The patch was written [1] but abandoned and never pushed [2]. It seems quite simple, though, so we could finish that up, if desired. The other approach is to disable line length checking in flake8 and use generic ArcanistTextLinter [3] for it, it's even mentioned in the docs [4]. That should be less work.

[1] https://secure.phabricator.com/D10512
[2] https://github.com/phacility/arcanist/blob/master/src/lint/linter/ArcanistFlake8Linter.php
[3] https://github.com/phacility/arcanist/blob/master/src/lint/linter/ArcanistTextLinter.php
[4] https://secure.phabricator.com/book/phabricator/article/arcanist_extending_lint/#arcanisttextlinter

> 
>  - default to "no" on lint exceptions - the idea is to avoid spending
>    time debating whether an exception is worth it or not.

If I read this correctly as "we don't have a strict 0-exceptions policy, but they should be very rare and if you propose one, you should have a very good reason to do so", I think that's fine and we'll see how that works in the next months.

> 
>  - until we get the codebase compliant "if you touch a file, fix the
>    lint errors even if those errors are not part of what you're
>    changing"

Even though this will make our files a bit consistent (modified parts looking a bit different than unmodified parts), I don't mind much and I think that this approach is fine for the moment. Especially before we find out if we're happy with the new approach. It will also minimize the changes between develop and disposable-develop.


More information about the qa-devel mailing list