Add Sphinx Lint to pre-commit #1786

hugovk · 2025-01-14T07:32:52Z

Plus add a few other handy linters and bump GitHub Actions.

📚 Documentation preview 📚: https://python-packaging-user-guide--1786.org.readthedocs.build/en/1786/

webknjaz

It'd be nice to split this so it's obvious what triggered formatting..

hugovk · 2025-01-14T14:09:23Z

Sure, how's this?

nedbat · 2025-01-14T15:38:06Z

.pre-commit-config.yaml

+- repo: https://github.com/pre-commit/pygrep-hooks
+  rev: v1.10.0
+  hooks:
+    - id: rst-backticks


This is already mentioned below.

BTW, I also tried this check, and it was horrible, because it complained about every instance of single-backticked words in my .rst files. It's valid to use single backticks, it's just not code.

Thanks, I've removed the duplicate.

Yep, it's valid, although it's pretty common people intend to write code but forget they're writing RST and not MD, and it shows as italics instead.

Yeah, also some projects explicitly define its role to be the same (https://github.com/pytest-dev/pytest/blob/517b006/doc/en/conf.py#L61C1-L61C25), confusing a casual contributor even more...

webknjaz · 2025-01-14T18:53:44Z

.pre-commit-config.yaml

@@ -8,10 +8,11 @@ repos:
  - id: check-added-large-files
  - id: check-case-conflict
  - id: check-merge-conflict
-  - id: check-symlinks


What's the justification here?

Flagged by the check-hooks-apply hook. Shall I remove check-hooks-apply and re-add check-symlinks?

Flagged by the check-hooks-apply hook:

Check hooks apply to the repository......................................Failed - hook id: check-hooks-apply - exit code: 1 check-symlinks does not apply to this repository

Shall I remove check-hooks-apply and re-add check-symlinks?

Honesty, not sure. I was just trying to understand how different bits of the PR might be related to the declared goal and each other...

Ned ended up not using this check: #1786 (comment). Perhaps, we shouldn't either.

webknjaz · 2025-01-14T18:54:13Z

.pre-commit-config.yaml

@@ -8,10 +8,11 @@ repos:
  - id: check-added-large-files
  - id: check-case-conflict
  - id: check-merge-conflict
-  - id: check-symlinks
+  - id: check-json


Could you add explanation somewhere why one check replaces the other instead of being additional?

I'll take a guess based on what I saw: check-hooks-apply complains that check-symlinks doesn't apply if the repo has no symlinks.

Oh.. I wish that was more apparent.

Thinking about it... It's rather suboptimal: if we ever start having symlinks, they won't be checked after this patch because nobody will remember to re-add it.

I agree. But perhaps it means we don't want check-hooks-apply.

After trying it in coveragepy, I removed check-hooks-apply.

.pre-commit-config.yaml

webknjaz · 2025-01-14T18:55:50Z

.pre-commit-config.yaml

+- repo: meta
+  hooks:
+    - id: check-hooks-apply
+    - id: check-useless-excludes


Are these fast? If so, they should be closer to the beginning for responsiveness.

On their own:

$ time pre-commit run --all-files Check hooks apply to the repository......................................Passed Check for useless excludes...............................................Passed pre-commit run --all-files 0.23s user 0.07s system 95% cpu 0.315 total

Compared to another hook on its own:

$ time pre-commit run --all-files rst ``code`` is two backticks............................................Passed pre-commit run --all-files 0.15s user 0.05s system 71% cpu 0.283 total

The whole lot:

$ time pre-commit run --all-files rst ``code`` is two backticks............................................Passed check for added large files..............................................Passed check for case conflicts.................................................Passed check for merge conflicts................................................Passed check json...............................................................Passed check yaml...............................................................Passed fix end of files.........................................................Passed mixed line ending........................................................Passed fix requirements.txt.....................................................Passed trim trailing whitespace.................................................Passed codespell................................................................Passed Disallow improper capitalization.........................................Passed rst ``code`` is two backticks............................................Passed rst directives end with two colons.......................................Passed rst ``inline code`` next to normal text..................................Passed ruff.....................................................................Passed ruff-format..............................................................Passed Sphinx Lint..............................................................Passed Check hooks apply to the repository......................................Passed Check for useless excludes...............................................Passed pre-commit run --all-files 4.24s user 1.08s system 276% cpu 1.923 total

The whole lot without them:

$ time pre-commit run --all-files rst ``code`` is two backticks............................................Passed check for added large files..............................................Passed check for case conflicts.................................................Passed check for merge conflicts................................................Passed check json...............................................................Passed check yaml...............................................................Passed fix end of files.........................................................Passed mixed line ending........................................................Passed fix requirements.txt.....................................................Passed trim trailing whitespace.................................................Passed codespell................................................................Passed Disallow improper capitalization.........................................Passed rst ``code`` is two backticks............................................Passed rst directives end with two colons.......................................Passed rst ``inline code`` next to normal text..................................Passed ruff.....................................................................Passed ruff-format..............................................................Passed Sphinx Lint..............................................................Passed pre-commit run --all-files 3.98s user 1.02s system 300% cpu 1.666 total

@hugovk

Tip
Pro tip: you can add -vv to pre-commit run and it will print out per-hook timings verbosely. It will be a bit more accurate.

But yeah, the idea here is that when the slower checks are running first, it may contribute to the feeling that it's stuck, while it's nicer the other way around. With that in mind, I'd probably put these at the beginning.

Thanks for the tip!

❯ time pre-commit run --all-files -vv rst ``code`` is two backticks............................................Passed - hook id: rst-backticks - duration: 0.09s check for added large files..............................................Passed - hook id: check-added-large-files - duration: 0.08s check for case conflicts.................................................Passed - hook id: check-case-conflict - duration: 0.07s check for merge conflicts................................................Passed - hook id: check-merge-conflict - duration: 0.06s check json...............................................................Passed - hook id: check-json - duration: 0.02s check yaml...............................................................Passed - hook id: check-yaml - duration: 0.05s fix end of files.........................................................Passed - hook id: end-of-file-fixer - duration: 0.05s mixed line ending........................................................Passed - hook id: mixed-line-ending - duration: 0.05s fix requirements.txt.....................................................Passed - hook id: requirements-txt-fixer - duration: 0.02s trim trailing whitespace.................................................Passed - hook id: trailing-whitespace - duration: 0.05s codespell................................................................Passed - hook id: codespell - duration: 0.13s Disallow improper capitalization.........................................Passed - hook id: disallow-caps - duration: 0.07s rst ``code`` is two backticks............................................Passed - hook id: rst-backticks - duration: 0.06s rst directives end with two colons.......................................Passed - hook id: rst-directive-colons - duration: 0.06s rst ``inline code`` next to normal text..................................Passed - hook id: rst-inline-touching-normal - duration: 0.07s ruff.....................................................................Passed - hook id: ruff - duration: 0.04s All checks passed! ruff-format..............................................................Passed - hook id: ruff-format - duration: 0.01s 2 files left unchanged Sphinx Lint..............................................................Passed - hook id: sphinx-lint - duration: 0.3s No problems found. Check hooks apply to the repository......................................Passed - hook id: check-hooks-apply - duration: 0.09s Check for useless excludes...............................................Passed - hook id: check-useless-excludes - duration: 0.08s pre-commit run --all-files -vv 4.18s user 1.05s system 292% cpu 1.788 total

I've moved them to the beginning.

webknjaz reviewed Jan 14, 2025

View reviewed changes

hugovk added 4 commits January 14, 2025 16:07

Bump GitHub Actions

df7ef72

Add Spinx Lint and extra linters to pre-commit

6339a91

Apply requirements-txt-fixer

e2a552e

Fix tab to spaces, found by Sphinx Lint

08107d5

hugovk force-pushed the sphinx-lint branch from b2fa066 to 08107d5 Compare January 14, 2025 14:09

nedbat reviewed Jan 14, 2025

View reviewed changes

Remove duplicate

eb1cf39

webknjaz reviewed Jan 14, 2025

View reviewed changes

.pre-commit-config.yaml Show resolved Hide resolved

webknjaz reviewed Jan 14, 2025

View reviewed changes

Move fast meta hooks to the beginning

983d39f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Sphinx Lint to pre-commit #1786

Add Sphinx Lint to pre-commit #1786

hugovk commented Jan 14, 2025 •

edited by github-actions bot

Loading

webknjaz left a comment

hugovk commented Jan 14, 2025

nedbat Jan 14, 2025

nedbat Jan 14, 2025

hugovk Jan 14, 2025

webknjaz Jan 14, 2025

webknjaz Jan 14, 2025

hugovk Jan 14, 2025

hugovk Jan 14, 2025

webknjaz Jan 14, 2025

webknjaz Jan 14, 2025

webknjaz Jan 14, 2025

nedbat Jan 14, 2025

webknjaz Jan 14, 2025

webknjaz Jan 14, 2025

nedbat Jan 14, 2025

nedbat Jan 14, 2025

webknjaz Jan 14, 2025

hugovk Jan 14, 2025

webknjaz Jan 14, 2025

hugovk Jan 14, 2025

Add Sphinx Lint to pre-commit #1786

Are you sure you want to change the base?

Add Sphinx Lint to pre-commit #1786

Conversation

hugovk commented Jan 14, 2025 • edited by github-actions bot Loading

webknjaz left a comment

Choose a reason for hiding this comment

hugovk commented Jan 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hugovk commented Jan 14, 2025 •

edited by github-actions bot

Loading