Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
JulienPalard merged 5 commits intopython:mainfromJulienPalard:mdk-sphinx-lint
Feb 10, 2022

Conversation

JulienPalard
Copy link
Member

@JulienPalardJulienPalard commentedFeb 3, 2022
edited by bedevere-bot
Loading

@arhadthedev
Copy link
Member

arhadthedev commentedFeb 3, 2022
edited
Loading

This PR touches a bunch ofMisc/NEWS.d/next/Build/*.rst changing nothing visible (exceptone insertion of a missing colon). Is it intended? Looks like spurious CR/LF conversion slipped in.

@@ -7,6 +7,8 @@ sphinx==4.2.0

blurb

sphinx-lint
Copy link
Contributor

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).

Copy link
Contributor

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).

Copy link
MemberAuthor

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.

Copy link
Contributor

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.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

OK for< 1, done.

@JulienPalard
Copy link
MemberAuthor

This PR touches a bunch ofMisc/NEWS.d/next/Build/*.rst changing nothing visible (exceptone insertion of a missing colon). Is it intended? Looks like spurious CR/LF conversion slipped in.

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.

arhadthedev reacted with thumbs up emoji

JulienPalard pushed a commit to sphinx-contrib/sphinx-lint that referenced this pull requestFeb 3, 2022
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.
@JulienPalard
Copy link
MemberAuthor

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.

@JulienPalardJulienPalard merged commitb878b3a intopython:mainFeb 10, 2022
@JulienPalardJulienPalard deleted the mdk-sphinx-lint branchFebruary 10, 2022 07:59
sobolevn added a commit to sobolevn/cpython that referenced this pull requestFeb 10, 2022
miss-islington pushed a commit that referenced this pull requestFeb 10, 2022
`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
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@jeanasjeanasjeanas left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@JulienPalard@arhadthedev@jeanas@the-knights-who-say-ni@bedevere-bot

[8]ページ先頭

©2009-2025 Movatter.jp