Skip to content

gh-109408: Add the docs whitespace check from patchcheck to pre-commit #109854

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hugovk merged 12 commits into from
Oct 10, 2023
+13 −33
Merged

gh-109408: Add the docs whitespace check from patchcheck to pre-commit #109854

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
dff437b
Run pre-commit as part of 'make patchcheck'
hugovk Sep 20, 2023
97c6ccf
Use f-strings
hugovk Sep 20, 2023
029a9a4
Add whitespace
hugovk Sep 20, 2023
d7690a3
Properly pluralise file(s), fix Unix typo
hugovk Sep 20, 2023
5cfcd19
Remove normalize_docs_whitespace from patchcheck, it's covered by pre…
hugovk Sep 20, 2023
1eb8042
Include inc files in pre-commit's trailing-whitespace
hugovk Sep 20, 2023
a85cd1c
Fix string
hugovk Sep 25, 2023
433153d
Use "s" variable for readability
hugovk Sep 25, 2023
a429344
Reinstate docs_modified
hugovk Sep 25, 2023
63c6d17
Reinstate doc_files for docs_modified
hugovk Sep 26, 2023
05a5d35
Revert makefile changes
hugovk Sep 27, 2023
f4b16ec
Merge branch 'main' into patchcheck-docs-whitespace
hugovk Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Run pre-commit as part of 'make patchcheck'
  • Loading branch information
@hugovk
hugovk committed Sep 25, 2023
commit dff437b42ca53f884444e87f90ba3118e26b5517
2 changes: 2 additions & 0 deletions Makefile.pre.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2772,6 +2772,8 @@ funny:
.PHONY: patchcheck
patchcheck: all
$(RUNSHARED) ./$(BUILDPYTHON) $(srcdir)/Tools/patchcheck/patchcheck.py
@$(RUNSHARED) ./$(BUILDPYTHON) -m pre_commit --version >/dev/null 2>&1 || $(RUNSHARED) ./$(BUILDPYTHON) -m pip install pre-commit >/dev/null 2>&1
$(RUNSHARED) ./$(BUILDPYTHON) -m pre_commit run --all-files
Copy link
Copy Markdown
Member

@AA-Turner AA-Turner Sep 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think make patchcheck should run pre commit, especially if the goal is to remove it. Hence I'd say this is a docs point to communicate.

Suggested change
@$(RUNSHARED) ./$(BUILDPYTHON) -m pre_commit --version >/dev/null 2>&1 || $(RUNSHARED) ./$(BUILDPYTHON) -m pip install pre-commit >/dev/null 2>&1
$(RUNSHARED) ./$(BUILDPYTHON) -m pre_commit run --all-files

Copy link
Copy Markdown
Member Author

@hugovk hugovk Sep 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking it would be a good idea to add a make lint target like we have for the PEPs repo (and Pillow), which seems to work there.

Perhaps we should add make lint now and have it run pre-commit; and maybe also patchcheck?

Copy link
Copy Markdown
Member

@AA-Turner AA-Turner Sep 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd agree. We should also update the devguide (currently only patchcheck is mentioned)

Copy link
Copy Markdown
Member Author

@hugovk hugovk Sep 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the Makefile changes from this PR, let's decide what to later after discussion:

https://discuss.python.org/t/do-you-use-make-patchcheck/34743?u=hugovk


.PHONY: check-limited-abi
check-limited-abi: all
Expand Down