Add Sphinx Lint to pre-commit #1786
Conversation
|
Sure, how's this? |
.pre-commit-config.yaml
Outdated
| - repo: https://github.com/pre-commit/pygrep-hooks | ||
| rev: v1.10.0 | ||
| hooks: | ||
| - id: rst-backticks |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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...
| @@ -8,10 +8,11 @@ repos: | |||
| - id: check-added-large-files | |||
| - id: check-case-conflict | |||
| - id: check-merge-conflict | |||
| - id: check-symlinks | |||
There was a problem hiding this comment.
What's the justification here?
There was a problem hiding this comment.
Flagged by the check-hooks-apply hook. Shall I remove check-hooks-apply and re-add check-symlinks?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
Ned ended up not using this check: #1786 (comment). Perhaps, we shouldn't either.
| @@ -8,10 +8,11 @@ repos: | |||
| - id: check-added-large-files | |||
| - id: check-case-conflict | |||
| - id: check-merge-conflict | |||
| - id: check-symlinks | |||
| - id: check-json | |||
There was a problem hiding this comment.
Could you add explanation somewhere why one check replaces the other instead of being additional?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Oh.. I wish that was more apparent.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I agree. But perhaps it means we don't want check-hooks-apply.
There was a problem hiding this comment.
After trying it in coveragepy, I removed check-hooks-apply.
Plus add a few other handy linters and bump GitHub Actions.
📚 Documentation preview 📚: https://python-packaging-user-guide--1786.org.readthedocs.build/en/1786/