Discussion about adding a rule for line-lengths

[grdw]

This issue is meant as a discussion but fairly often I see code in WeTransfer projects (specifically spaceship, because I mainly work in spaceship) with fairly long line lengths (Examples: [1], [2] and there are a lot of examples to be honest) and personally I am not a really big fan.

It makes code fairly hard to read and line-wrapping in and editor makes it even harder to read.

There are a couple of solutions:

1. Add a rubocop rule enforcing a line length
2. People convince me that I should not complain and maybe get a wider screen
3. Another solution?

I've added some people to take part in this discussion. If anybody knows other people within WeTransfer that feel strongly about the subject, please tag them in this issue.

[lorenzograndi4]

What's a standard max length? 80? 100? We could start enforcing it informally on spaceship. Unless @linkyndy is open to adding a rule to the gem.

[bermannoah]

80 is usually the standard, I think. For me it's always a question of "is it easier to parse as a full line or as a constantly descending block?"

...and I'm never sure which is the right answer. I don't really have an opinion here but will happily comply with whatever we decide.

[astrikos]

79 chars is a standard coming back from the old days when console was 80 char length. Nowdays we have big screens so it shouldn't be forced strictly :) I personally have my editor set to notify me when i reach 80 and my linter to complain when it gets to 100 Big projects also have stopped following 79 char rule in favor of readability https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/

An exception to PEP 8 is our rules on line lengths. Don’t limit lines of code to 79 characters if it means the code looks significantly uglier or is harder to read. We allow up to 119 characters as this is the width of GitHub code review; anything longer requires horizontal scrolling which makes review more difficult

Not sure what rails is doing though, maybe it's worth following their standards if they are sensible. that's just my 2 cents.

[linkyndy]

I'm in favour of short(er) lines. The ruby style guide feels the same.

The reason I didn't add this rubocop rule was because it would cause a lot of manual labour to fix the generated offences.

If we find a way not to mess up everybody's feng-shui, I'm definitely up for this!

[grdw]

If we find a way not to mess up everybody's feng-shui, I'm definitely up for this!

Yes, adding this rule will cause a lot of Travis builds to go boom. So maybe add it as a warning first, than give everybody a month (or a week, or 2 months or if anybody can give a better guestimate 😅 ) to fix it and after that time, enforce it as a rule. Does that sound as a good process?

[lorenzograndi4]

It could be. Which apps are using this gem atm? I can help with this change if we leave /frontend out of it 😄

[linkyndy]

Frontend is not there atm, so we're ok-ish. format_parser, zip_tricks have it, don't know about the rest. But it should be pretty straightforward to fix offences in these ones.

[arnoFleming]

@WeTransfer/backend: Do we want to release a new version that actualy breaks on too long lines?

[linkyndy]

I wouldn't now...it's not something that Rubocop can automatically fix, so it might cause some hassle for other people to comply with. But I certainly would like to see this in place!

[grdw]

[..] so it might cause some hassle for other people to comply with

Yes. It is some hassle to comply with. Spaceship did it step by step. So first get rid of all the lines > 200 characters, than 175, 150 etc. This is a pretty large time effort though with absolutely no user impact. However, it does have positive developer impact 😅

[lorenzograndi4]

I wouldn't make it break right away. An approach for this could be to start reducing line length and only enforce it on wt_style once all the repos are compliant. Do we have a list of repos currently using wt_style?