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

Status: NEW

(Firefox Build System :: Bootstrap Configuration, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

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

Comment 1

[Mark Banner (:standard8)]

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.

Comment 2

[Mark Banner (:standard8)]

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.

Comment 3

[Mark Banner (:standard8)]

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.

Comment 4

[Mark Banner (:standard8)]

We should also make this work for bootstrap, rather than mercurial-setup, so that it works for git repos as well.

Comment 5

[Andrew Halberstadt [:ahal]]

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.

Comment 6

[Andrew Halberstadt [:ahal]]

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.

Comment 7

[Andrew Halberstadt [:ahal]]

Or alternatively we can block on that bug and add this to |mach git-setup|

Comment 8

[Mark Banner (:standard8)]

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).

Comment 9

[Andrew Halberstadt [:ahal]]

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.