Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
bpo-42238: [doc] moving from rstlint.py to sphinx-lint.#31097
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
arhadthedev commentedFeb 3, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 insphinx-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 besphinx-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
to72f8db8
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` CPythonbranch:```https://github.com/python/cpython/blob/main/Doc/tools/rstlint.py ^^^^```However, afterpython/cpython#31097 is merged,the link will stop working. To keep the link working, `main` needs to bereplaced 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
to9b02019
Compare`main` branch is failing, seehttps://dev.azure.com/python/cpython/_build/results?buildId=96616&view=logs&j=4db1505a-29e5-5cc0-240b-53a8a2681f75&t=a975920c-8356-5388-147c-613d5fab0171Logs:```PATH=./venv/bin:$PATH sphinx-lint -i tools -i ./venv -i README.rstNo 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:#31097CC@JulienPalardAutomerge-Triggered-By: GH:JulienPalard
Uh oh!
There was an error while loading.Please reload this page.
https://bugs.python.org/issue42238