Don't run mozlint taks for beta, release or esr
Categories
(Developer Infrastructure :: Lint and Formatting, task)
Tracking
(Not tracked)
People
(Reporter: Sylvestre, Assigned: Sylvestre)
References
(Blocks 1 open bug)
Details
Attachments
(1 obsolete file)
For most of the patches landing in beta, release or esr, we will run mozlint on them.
However:
- during the review phase, linters are executed and should have been fixed by developers
- as the patches should land in autoland/m-c before being uplifted, they already had linting running on them
- esr is by definition an older code base, m-c is in general more green than esr, we won't decrease the quality
- we run older tools on esr
- I don't remember if it ever produces anything interesting in an uplift
Comment 1•4 years ago
|
||
Are you intending on disabling all or some of them?
Have you considered about handling of bitrot situations or custom uplifts where the patch had to be written differently?
Assignee | ||
Comment 2•4 years ago
|
||
Are you intending on disabling all or some of them?
All for now but we could enable a few
Have you considered about handling of bitrot situations or custom uplifts where the patch had to be written differently?
Did that ever happen ? Feels like rhetorical for now
Comment 3•4 years ago
|
||
(In reply to Sylvestre Ledru [:Sylvestre] from comment #2)
Have you considered about handling of bitrot situations or custom uplifts where the patch had to be written differently?
Did that ever happen ? Feels like rhetorical for now
Ever? Empirically, yes. Is it common now? Unknown. Ryan?
Comment 4•4 years ago
|
||
I've definitely seen ESR uplifts hit linting issues due to differing rules between the branches.
Comment 5•4 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #4)
I've definitely seen ESR uplifts hit linting issues due to differing rules between the branches.
That's an interesting data point, too. I was asking how common it was for people to have to custom-write patches for uplifts nowadays. :)
Comment 6•4 years ago
|
||
It's late in the ESR cycle, most patches need some form of rebasing at this point. Entirely different patches? Pretty uncommon I'd say.
Comment 7•4 years ago
|
||
I think my biggest concern would be for JavaScript code at least. It is easy to make trivial mistakes and for them not to be noticed, and test coverage isn't always 100%. c++ / rust are better, as they would at least catch things like unused/undefined variables.
I don't have any data on this relative to uplifts/esr though.
One thing that might also be viable, is to disable prettier (aka eslint formatting) on those branches to cut down the run time and reduce potential for back-outs/re-lands due to formatting only changes. We'd probably want to check the effective rules that get disabled, but I think you could get away with it.
Assignee | ||
Comment 8•4 years ago
|
||
yeah, it doesn't have to be black/white. We could keep eslint and maybe flake8
I've definitely seen ESR uplifts hit linting issues due to differing rules between the branches.
We have different kind of lintings. undefined variable is a big deal, missing "," or too many spaces is something else :)
Assignee | ||
Comment 9•4 years ago
|
||
We keep eslint and yaml on all projects as they could be usefull during uplift processes.
Updated•4 years ago
|
Assignee | ||
Comment 10•4 years ago
|
||
So, i have been bikeshedding here.
According to a dashboard, we are spending a very small amount on this for all repo (less than US$ 4k since the beginning of 2020).
So, if one day, a linter catch an actual issue in m-r or esr, the early catching of the error will be worth all the money we had spend for it.
Updated•2 years ago
|
Description
•