`mach bootstrap` should offer to enable the tools/lint/hooks.py mozlint hook

NEW
Unassigned

Status

3 years ago
3 months ago

People

(Reporter: jaws, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

eslint runs on the build server and will fail if eslint errors are introduced. Developers who work on frontend code should have this commit hook enabled, so we should offer it during mercurial-setup.

It is already part of the repo at /tools/mercurial/eslintvalidate.py
After I've got bug 1360595 landed, I'll take a look at this and see if I can work out something for Mercurial and for git.
Assignee: nobody → standard8
Depends on: 1360595
At the moment I think this really depends on bug 1361972 - changing how the existing hooks work to provide simpler hooks overall.

I'm unassigning myself for now, though I'd really like this to be done as soon as that one is done.
Assignee: standard8 → nobody
Depends on: 1361972
Summary: `mach mercurial-setup` should offer to enable the mozeslint.py commit hook → `mach mercurial-setup` should offer to enable the tools/lint/hooks.py mozlint hook
Andrew, any chance you (or someone) be able to help out (or take this on) with this? I started playing around, but then realised I'll need to deal with git and hg, and wasn't quite sure of the ideal structure for this or how long it'd take me.
Flags: needinfo?(ahalberstadt)
We should also make this work for bootstrap, rather than mercurial-setup, so that it works for git repos as well.
Summary: `mach mercurial-setup` should offer to enable the tools/lint/hooks.py mozlint hook → `mach bootstrap` should offer to enable the tools/lint/hooks.py mozlint hook
Ideally we'd have a single |mach vcs-setup| that handled both hg and git. Or maybe even just merge it into |mach bootstrap| entirely. But that's scope bloat for this bug, I guess putting this in bootstrap for now makes sense (gps might have opinions).

I'm definitely happy to help out, though I don't think I'll have time to own it at this moment. I'll also ask around if anyone would be interested in taking this. Fwiw, I've also never modified mozboot, so I'm not a suitable reviewer. Gps would be the main owner, though he's on pto for a few weeks. I've asked on #build who the next best reviewer would be.
Component: mach → Build Config
Flags: needinfo?(ahalberstadt)
See Also: → bug 1257478
Looks like from that bug gps was leaning towards merging everything into |mach bootstrap|. So I guess putting this setup in there would make sense.
Or alternatively we can block on that bug and add this to |mach git-setup|
I was pondering about having a ./mach lint --setup, and then having that be called by bootstrap (since it'd be nice if eslint was also setup when bootstrap happens).
I like that! Each linter could define its own formal "setup" function that gets called on |mach lint --setup|.

Would also give us place to implement this stuff without needing to worry about mercurial-setup/git-setup/bootstrap refactorings. After we get it implemented we can call it from the appropriate places depending on what happens in bug 1257478.
Depends on: 1429457

Updated

a year ago
Product: Core → Firefox Build System
Duplicate of this bug: 1239140

Updated

10 months ago
Component: General → Bootstrap Configuration
See Also: → bug 1511904
You need to log in before you can comment on or make changes to this bug.