-
-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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.
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
bpo-42238: [doc] moving from rstlint.py to sphinx-lint. #31097
Conversation
This PR touches a bunch of |
Doc/requirements.txt
Outdated
@@ -7,6 +7,8 @@ sphinx==4.2.0 | |||
|
|||
blurb | |||
|
|||
sphinx-lint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this dependency be pinned? I say this because if we refactor sphinx-lint (e.g. as a Sphinx builder as discussed in sphinx-contrib/sphinx-lint#1), it will require adaptation here. A new release will have to be made with the changes before they can be used in CPython, and between the release and the actual switch, things will be broken (plus people might not understand what's going on it if they don't pull recent changes but install a newer sphinx-lint).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thought: it will make it easier to add checks that might have a few false positives (preferably there won't be any, but it could be tricky to eliminate some rare cases).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will require adaptation here.
I don't think so, but I may miss something. sphinx-lint should be able to find the conf.py by itself, and we'll be able to add a sphinx install dependency, so it looks like it should be doable without adaptation.
Still we can pin <1
and use semver, so if we break something we can bump the major version if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The normal way to invoke a Sphinx builder would basically be sphinx-build -b lint
. I haven't investigated if it's feasible to allow direct invocation by a script; there would be implications on which Sphinx to use for example (from some venv or not?). I think it can't hurt to require <1
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK for < 1
, done.
07ba286
to
72f8db8
Compare
This is intended: I edited those manually, that's because while moving rstlint.py to sphinx-lint I also worked on it for a few month and added a few other checks. The "invisible" issue was missing newlines at end of file, it was only found in 17 rst files (over 103 files in NEWS.d/next/). No CR/LF issues found. |
README.md links to rstlint.py in a dynamic head of `main` CPython branch: ``` https://github.com/python/cpython/blob/main/Doc/tools/rstlint.py ^^^^ ``` However, after python/cpython#31097 is merged, the link will stop working. To keep the link working, `main` needs to be replaced with some concrete commit SHA.
I « realized » that even if the script is hidden under Doc/tools, it exist, and as so it is used by someone somewhere, so instead of removing the file I just added a warning to it. |
c9d51e6
to
9b02019
Compare
`main` branch is failing, see https://dev.azure.com/python/cpython/_build/results?buildId=96616&view=logs&j=4db1505a-29e5-5cc0-240b-53a8a2681f75&t=a975920c-8356-5388-147c-613d5fab0171 Logs: ``` PATH=./venv/bin:$PATH sphinx-lint -i tools -i ./venv -i README.rst No problems found. PATH=./venv/bin:$PATH sphinx-lint ../Misc/NEWS.d/next/ [1] ../Misc/NEWS.d/next/Library/2022-02-09-00-53-23.[bpo-45863]().zqQXVv.rst:0: No newline at end of file (no-newline-at-end-of-file). [1] ../Misc/NEWS.d/next/Build/2022-01-19-11-08-32.[bpo-46430]().k403m_.rst:0: No newline at end of file (no-newline-at-end-of-file). 2 problems with severity 1 found. ``` This PR fixes these two problems, so `main` is green again. Related PR: #31097 CC @JulienPalard Automerge-Triggered-By: GH:JulienPalard
https://bugs.python.org/issue42238