Page MenuHomePhabricator

Decide on max-len rule for eslint-wikimedia-config
Closed, ResolvedPublic

Description

Our eslint config doesn't enforce line length in any way, resulting in inconsistent code, and comments, which hurts readability.

Manual:Coding_conventions/JavaScript#Line_length mentions 80-100 as a convention.

The JavaScript community at large has standardized at 80 chars length, some examples:

Although there are some famous presets that use 100:


In addition, we should special case some things like //eslint-disable* comments, and QUnit lines which tend to run over because they contain a long string. Other things like lines with a string, template literals, long urls, ... could also be taken into account, see example

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 19 2018, 10:04 AM
Jhernandez renamed this task from Enforce 80 characters line length with eslint in eslint-wikimedia-config to Enforce 80-100 characters line length with eslint in eslint-wikimedia-config.Jan 19 2018, 10:04 AM

I wasn't sure how to tag this so I just added JavaScript.

Change 405037 had a related patch set uploaded (by Jhernandez; owner: Jhernandez):
[mediawiki/extensions/Popups@master] Limit line length to 80 characters

https://gerrit.wikimedia.org/r/405037

Change 405037 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Limit line length to 80 characters

https://gerrit.wikimedia.org/r/405037

Made an initial PR: https://github.com/wikimedia/eslint-config-wikimedia/pull/63 to show how this could look like.

Why 80 for JS when phpcs currently enforces 100 for PHP?

Yeah, I think 100 is more reasonable.

I personally don't have any preference. It is a subjective decision.

I personally have a small laptop, and shorter lines usually help me while reviewing on gerrit side-by-side and developing, which is why I chose 80 since the docs say 80-100. But it doesn't really matter as long as it is enforced.

I marked the PR as editable so that any maintainers can change it, and I'm happy to change it too. Let me know if so!

How do we want to decide on this, have a vote in Front-end-Standards-Group, or something else? 👍

Linting at 100 makes sense to me.

From IRC:

<Volker_E> 80
<Volker_E> and amend phpcs :P
<Krinkle> We can always reduce to 80 later if deemed useful. I think we should start with 100. It will median around 70-80 regardless.

👍🏻

Krinkle renamed this task from Enforce 80-100 characters line length with eslint in eslint-wikimedia-config to Decide on max-len rule for eslint-wikimedia-config.Mar 20 2018, 11:42 PM

Change 420941 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/extensions/Popups@master] Tooling: Remove some redundant max-len patterns

https://gerrit.wikimedia.org/r/420941

Change 420941 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Tooling: Remove some redundant max-len patterns

https://gerrit.wikimedia.org/r/420941

Thanks for the patches. We will remove our 80 len and 2 tabwidth settings in Popups when the PR is merged and a version is released. It is great to see this moving along!