Link to devguide when lint fails

[nineteendo]

Feature or enhancement

Proposal:

Could we automatically post this message when lint.yml fails? If we explain how to set it up, this could save unnecessary CI churn.

Please set up pre-commit: https://devguide.python.org/getting-started/setup-building/#install-pre-commit-as-a-git-hook

Links to previous discussion of this feature:

• https://discuss.python.org/t/linter-settings-for-cpython-pull-requests-in-vs-code/50645/11
• Add pre-commit setup to the Lifecycle of a pull request devguide#1305

[hugovk]

If you look at a failure on the CI, it says:

If you are seeing this message in CI, reproduce locally with: pre-commit run --all-files.
To run pre-commit as part of git workflow, use pre-commit install.

https://github.com/python/cpython/actions/runs/8896709778/job/24429952258#step:4:149

[nineteendo]

It doesn't always do that. e.g. https://github.com/python/cpython/actions/runs/8904385748/job/24453579843#step:4:133
It also doesn't mention what pre-commit is, and there are still people that haven't set it up: https://github.com/python/cpython/actions/workflows/lint.yml?query=is%3Afailure

[nineteendo]

pablogsal has failed it 87 times: https://github.com/python/cpython/actions/workflows/lint.yml?query=is%3Afailure+actor%3Apablogsal

Edit: 88 times

[pradyunsg]

It seems pre-commit will print that message when the hooks have modified files, but not if the repository has files that cause one of the linters to fail/find issues.

[pradyunsg]

It's perfectly acceptable to not set up pre-commit, and only use it as a linter runner. Ensuring that code failing lint checks is not checked in to main is what the CI checks are for.

[hugovk]

Yep, some people really don't running pre-commit locally, and that's totally fine!

I'm a fan and recommend it, but I'm not sure we should be telling others to use it for each failing commit.

[nineteendo]

How about once per pull request? Or could we save user preferences? 2.7% of all commits fail this workflow, which should be lowered.

[hugovk]

Even once per PR will be annoying for people who don't want to use pre-commit.

Why should 2.7% be lowered? It already feels low to me.

The lint workflow is one of our quickest (<30s), it's not causing delays for others (compare our limited macOS capacity that sometimes causes backed up builds and an hour+ of waiting) and it's doing its job when it fails.

[nineteendo]

Hmm, could we cancel the other workflows when lint fails? Leaving them running wastes capacity that could be used for other pull requests. This wouldn't be as annoying as my previous proposal: python/cpython#117580

[JelleZijlstra]

That proposal would make life harder for me as a contributor since I'd have to wait longer to see if any tests are failing. With the current flow, I could wait for all CI to run, then fix both lint and test issues in one push. With your proposal, I'd have to fix lint, push, then maybe push again if a test is broken.

I don't think we need to change anything here.

[nineteendo]

It has only failed 9 times for you (1.7%): https://github.com/python/cpython/actions/workflows/lint.yml?query=is%3Afailure+actor%3AJelleZijlstra
And how about other people that need to wait longer to see if their tests are failing?

[nineteendo]

Not convinced? OK then I'll close this.