1738728 - Automate linting/formatting prior to moz-phab submission

Status: NEW

(Developer Infrastructure :: Lint and Formatting, enhancement)

Description

[Alex Hochheiden [:ahochheiden] [he/him]]

The reviewbot on Phabricator reports linting/formatting that needs to be done and gives the developer a command to run the analysis.

Example: https://phabricator.services.mozilla.com/D129063#4199548

To do the actual fix for that example, I just run ./mach lint -l black --fix <relative path>, but I feel like this could be done automatically before it ever makes it on to Phabricator. Either as a pre-commit hook, or as a part of the moz-phab submission.

It seems like it would just be a trivial amount of overhead/time added to a commit/submission (a few seconds?), and save developer time later down the line during the review process (minutes?) since they wouldn't have to run the linting/formatting commands manually and --amend their commits.

Comment 1

[Shivam Singhal [ :championshuttler ]]

this reminds me of https://ahal.ca/blog/2021/hide-your-lint-errors/

Comment 2

[Andrew Halberstadt [:ahal]]

To fix this we'll either need to:

1. Provide a wrapper around moz-phab
2. Have mach bootstrap add an alias to the user's $SHELL
3. Implement a way in moz-phab to run hooks on submission (and then still have mach bootstrap set the hook up)

Also the overhead of mach lint is unfortunately not super trivial (especially if a lint config file is modified and we lint the entire tree), so this would have to be a push hook not commit.

Comment 3

[Mike Hommey [:glandium]]

We have a bunch of mercurial extensions that do parts of this for js and C++, but at commit time: https://hg.mozilla.org/hgcustom/version-control-tools/file/0900473a332f7136cf59277c874145606758e345/hgext (see js-format, clang-format, format-source)