Now we have hooks for ESLint, I think it would be quite easy to extend them for flake8 (or add new ones). This would help python devs to see issues before they land.

Mozlint has a --outgoing parameter which is supposed to run all linters only against the files touched in commits that would get pushed to mozilla-central. It's supposed to work with git too, but someone recently reported trouble getting it working with git-cinnabar. Assuming we iron out any remaining bugs with git-cinnabar, I'd love if instead of having a different hook for each linter, we just had a single hook that ran: ./mach lint --outgoing I think doing this would also be simpler than the existing eslint hooks from a code perspective. (Note: there are some potential rust and cpp linters coming down the pipeline soon, so using |mach lint --outgoing| would also automatically add those)

Ok, I think I'm happy with that idea, lets go with that for now, I think we can re-work this bug. I see the git-cinnabar issue as well, so I'll get that filed.

Oh, git-cinnabar is bug 1369787.

This patch is almost ready to land, and should obsolete the eslintvalidate hooks (both git and hg). Instead of removing the old hooks, we could make them print an error message including the upgrade path to this new hook. I'd prefer to do this in a follow-up bug in a little while though (after these new hooks have had some more use). Before landing this patch I need to: * Support a mode for only linting staged changes on git * Test the push hook on git (don't have cinnabar setup on this laptop)

Comment on attachment 8882768 [details] Bug 1361972 - Add a pre-push and pre-commit mozlint hooks https://reviewboard.mozilla.org/r/153876/#review159874 I'm a little concerned that we're not using the exact diffs according to the upstream we are actually pushing to, but I think this will be OK for the majority of cases. One thing I did spot is that not each individual commit will necessarily pass lint - but I suspect that would be hard to enforce anyway, and not really much benefit given the current workflows. So r=Standard8, there's just one bit that we do want to change to maintain consistency with the existing pre-commit hooks. ::: tools/lint/hooks.py:35 (Diff revision 3) > + return run_mozlint(hooktype, kwargs.get('pats', [])) > + > + > +def git(args=sys.argv[1:]): > + hooktype = os.path.basename(__file__) > + return run_mozlint(hooktype, args[:1]) I think returning the result in the 'push' case is correct - we want that to hard fail. For the 'commit' case, I think we should always return success but just dumping the output (for both hooks). See the discussion on bug 1360595 comment 6 / 7 and bug 1230300 comment 4.

Comment on attachment 8882638 [details] Bug 1361972 - [mozlint] Add ability to only lint staged changes to --workdir with git https://reviewboard.mozilla.org/r/153728/#review159876

Comment on attachment 8882768 [details] Bug 1361972 - Add a pre-push and pre-commit mozlint hooks https://reviewboard.mozilla.org/r/153876/#review159874 Could you elaborate on the first part? If I run `hg push review`, then `review` is passed into the hook via the `pats` arg so the final command becomes `mach lint --outgoing review`. Similarly git will pass in the push destination via `sys.argv`. Have you noticed it linting against a different remote? If so, I'd like to fix that. On the second point, yeah that would be difficult to implement. Plus the taskcluster tasks won't prevent that anyway, so seems not worth pursuing here. > I think returning the result in the 'push' case is correct - we want that to hard fail. > > For the 'commit' case, I think we should always return success but just dumping the output (for both hooks). > > See the discussion on bug 1360595 comment 6 / 7 and bug 1230300 comment 4. Makes sense!

Comment on attachment 8882768 [details] Bug 1361972 - Add a pre-push and pre-commit mozlint hooks https://reviewboard.mozilla.org/r/153876/#review159874 Aha, ok, I hadn't spotted the parts arg. So it probably will work as we want it to :-)

Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/55b72ade8f20 [mozlint] Add ability to only lint staged changes to --workdir with git r=standard8 https://hg.mozilla.org/integration/autoland/rev/ea6fb7712a57 Add a pre-push and pre-commit mozlint hooks r=standard8

https://hg.mozilla.org/mozilla-central/rev/55b72ade8f20 https://hg.mozilla.org/mozilla-central/rev/ea6fb7712a57