On Tue, 16 Jun 2015 06:25:22 -0400 (EDT)
Kamil Paral <kparal(a)redhat.com> wrote:
> 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.
Yeah, 100 is fine by me. 120 was a somewhat arbitrary number that I
thought I remembered from previous conversations
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.
I think that we can work around this by putting config items in
setup.cfg or tox.ini like they talk about in that revision (and why the
author abandoned it, I think).
[1]
https://secure.phabricator.com/D10512
[2]
https://github.com/phacility/arcanist/blob/master/src/lint/linter/Arcanis...
[3]
https://github.com/phacility/arcanist/blob/master/src/lint/linter/Arcanis...
[4]
https://secure.phabricator.com/book/phabricator/article/arcanist_extendin...
>
> - 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.
That's pretty much what I meant, yeah.
Setting rigid "absolutely no exceptions ever" rules are generally a bad
idea. Unavoidable at times but there are usually problems that come
along with them.
>
> - 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.
Yeah, let's see how well this works out. Hopefully we'll be able to
merge disposable-develop back into develop soon and we won't have 2
active-ish branches to keep up to date.
Tim