gh-109408: Add the docs whitespace check from patchcheck to pre-commit#109854
gh-109408: Add the docs whitespace check from patchcheck to pre-commit#109854hugovk merged 12 commits into
Conversation
…-commit's trailing-whitespace
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
AlexWaygood
left a comment
There was a problem hiding this comment.
LGTM (though I can't review the Makefile changes :)
Sorry, something went wrong.
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
sobolevn
left a comment
There was a problem hiding this comment.
Right now Makefile does not have pip mentions at all, I think that adding a new target with external deps needs a bit of discussion.
Since Makefile has a biiig range of different use-cases.
In my opinion having pre-commit CLI and in CI on its own is good enough.
Sorry, something went wrong.
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
To clarify my opinion: I don't mind which of either of these are used, as I use Windows so the choice is personally irrelevant! I'll be happy with whatever Hugo goes for. A |
Sorry, something went wrong.
|
We could perhaps create a poll on Discourse in the core development category asking how many people actually run |
Sorry, something went wrong.
|
Written up some questions at https://discuss.python.org/t/do-you-use-make-patchcheck/34743 A |
Sorry, something went wrong.
|
Thanks @hugovk for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12. |
Sorry, something went wrong.
…-commit (pythonGH-109854) (cherry picked from commit 7426ed0) Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
…-commit (pythonGH-109854) (cherry picked from commit 7426ed0) Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
…e-commit (GH-109854) (#110595) gh-109408: Add the docs whitespace check from patchcheck to pre-commit (GH-109854) (cherry picked from commit 7426ed0) Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
…e-commit (GH-109854) (#110594) gh-109408: Add the docs whitespace check from patchcheck to pre-commit (GH-109854) (cherry picked from commit 7426ed0) Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
…-commit (python#109854) Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Moves one of the patchcheck checks to pre-commit.
1.
patchcheck's
normalize_docs_whitespaceruns on files wherefn.startswith('Doc') and fn.endswith(('.rst', '.inc')pre-commit's
trailing-whitespaceis already running onc,pythonandrstin the whole codebase.So this widens the check to cover
incfiles in the whole codebase, not just inDoc. I think this is a good thing.2.
patchcheck substitutes matches of
re.compile(br'\s+(\r?\n)$')withbr'\1. That is, it strips multiple trailing\rand\ncharacters.pre-commit's
trailing-whitespacetrims all trailing whitespace by default (docs). This also widens the check, I think this is a good thing.3.
Removedocs_modified(doc_files)from patchcheck, it merely printed out whether any docs files were modified. I don't think it's useful now patcheck does no docs checking, and I'd like to eventually remove patchcheck.4.
Some people might be still runningmake patchcheck? If so, to maintain some kind of parity, I've added steps to install and run pre-commit as part of the Makefile command.We're doing a similar thing in the PEPs repo:
https://github.com/python/peps/blob/a159a9ac58ca73dc1da0b626b6df77807af455d0/Makefile#L62-L77
(And Pillow: https://github.com/python-pillow/Pillow/blob/main/Makefile)
Does this seem useful, or better to leave it out?
5.
Finally, some cleanup of
patchcheck.py: whitespace, f-strings, consistent pluralised outputs, fix typo.