Introduce precommit hooks

[LeonarddeR]

Is your feature request related to a problem? Please describe.

Sometimes, people including myself forget to run unit tests or linting before a commit. There are also other tasks that can be automated at commit time.before

Describe the solution you'd like

Introduce precommit hooks. Note that the add-on template already has support for these. They can be used to run unit tests and linting before committing. They can also be used to automatically remove trailing white space, for example.

[CyrilleB79]

Pre-commit hooks are nice for add-ons; at least one that I have created for linting. If it lasts only 2-3 seconds to commit a file, it remains acceptable.

But for NVDA the problem is that the checks last more time due to the number of files:

• Unit tests take clearly too much time for a commit
• Linting check against master may be OK, but what if you develop a branch against beta or another branch?

I am not in favor of pre-commits that modify the full code files such as automatic linting or space removing, since as discussed in #15331 (review), it impacts the git blame history. In this case, I'd prefer a unique commit to perform the linting of the whole codebase (but that's not the direction taken by NV Access for now); this way, I can exclude this commit when I git blame.
Of course, a commit processing that would operate only on the code lines that you have already modified would be OK (if possible).

[seanbudd]

Closing for the reasons Cyrille has raised

[LeonarddeR]

I'm reopening this since the future of linting/formatting of NVDA is still open for discussion, see #15299. We can always close this again if there is a stronger consensus for it.

[CyrilleB79]

@LeonarddeR, even if NV Access can now be in favor of a linting of the whole codebase, the reasons why this issue was closed still stand:

• check time should be short
• linting a diff should be aware of the base branch

Have you some specific checks in mind for a pre-commit hook? If not, maybe the comments of this issue would rather deserve a GitHub discussion?

[LeonarddeR]

* check time should be short

Shouldn't be a problem when only linting changed files.

* linting a diff should be aware of the base branch

Linting a diff is no longer necessary when the baseline is correctly formatted and linted. Even when we'd lint a diff, the diff would be the diff of that commit alone, not against a base branch.

Have you some specific checks in mind for a pre-commit hook? If not, maybe the comments of this issue would rather deserve a GitHub discussion?

i actually agree that unit tests are probably way too much time consuming, but linting should certainly be possible and cause less issues. If there is a desire to convert line endings to a standardized format, we can add a line ending check, though ideally that'd be performed by the linter.

Note that pre-commit hooks would still be optional, we can't force them upon users anyway.

[seanbudd]

This has been converted to a discussion as we need a more specific implementation discussed - i.e. we need specific hooks defined, and the impact of these clear. If we are moving towards an auto-linting solution - auto linting changed files in the commit have a risky O(N)+ computation time. While N is generally small, there are exceptions i.e. when changing a popular function name, you'd be linting large numbers of files in the codebase. Linting unit tests is not ideal, as you may want to create intermediate commits where unit tests are broken.