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

Restore default role check inmake check.#92290

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

Conversation

@ezio-melotti
Copy link
Member

This PR restores the check for the default role (seeGH-92289) by doing two things. First, it sets theseverity to0 adding--severity=0, so that failing the default role check results in an error. This however also enables the long lines check, so it explicitly disabled it with--disable='line too long'. There are currently no other checks withseverity=0.

This PR only fixesMakefile. If this approach is sound,make.bat should be fixed too.

Using--enable instead might be a better approach, oncesphinx-contrib/sphinx-lint#27 lands.

As a side note, it took me a bit to figure out that--disable= expected a message rather than the name of a checker.

cc@JulienPalard

@ezio-melottiezio-melotti self-assigned thisMay 4, 2022
@ezio-melottiezio-melotti marked this pull request as draftMay 4, 2022 04:41
@ezio-melotti
Copy link
MemberAuthor

With this PR,the check successfully failed:

PATH=./venv/bin:$PATH sphinx-lint -i tools -i ./venv -i README.rst --severity=0 --disable='line too long'No problems found.PATH=./venv/bin:$PATH sphinx-lint ../Misc/NEWS.d/next/ --severity=0 --disable='line too long'[0] ../Misc/NEWS.d/next/Library/2022-04-26-18-02-44.gh-issue-91928.V0YveU.rst:1: default role used (hint:for inline code, use double backticks)[0] ../Misc/NEWS.d/next/Documentation/2022-04-24-22-09-31.gh-issue-91888.kTjJLx.rst:1: default role used (hint:for inline code, use double backticks)2 problems with severity 0 found.make:*** [Makefile:217: check] Error 1##[error]Bash exited with code '2'.

@ezio-melotti
Copy link
MemberAuthor

I added the same changes tomake.bat, but I don't have a Windows machine to test them (cc @python/windows-team).

make.bat seems outdated compared toMakefile, so I made a couple of changes to make them converge:

In addition,bfba2cd added thevenv dir to the ignored dirs, butmake.bat doesn't seem to use venv, so no change was required.

@vstinner
Copy link
Member

The CI fails:

Error: [0] ../Misc/NEWS.d/next/Documentation/2022-04-24-22-09-31.gh-issue-91888.kTjJLx.rst:1: default role used (hint: for inline code, use double backticks)Error: [0] ../Misc/NEWS.d/next/Library/2022-04-26-18-02-44.gh-issue-91928.V0YveU.rst:1: default role used (hint: for inline code, use double backticks)

@ezio-melotti
Copy link
MemberAuthor

Yes, I'm leaving the failures so that people can test if they are detected on Windows too. If they are, I'll fix them and merge the PR.

@AlexWaygood
Copy link
Member

AlexWaygood commentedMay 5, 2022
edited
Loading

Tried the patch out on my Windows machine:

C:\Users\alexw\coding\cpython>"Doc/make.bat" checkUsing py -3.10 (found 3.10 with py.exe)Error: path too does not existError: path too does not exist

(I'll be honest, I usually don't bother with the.bat file when I'm writing a docs patch. I usually usesphinx-build to build the docs locally,git diff to check for trailing whitespace, andsphinx-lint in CPython's CI to check for markup errors.)

@ezio-melottiezio-melotti mentioned this pull requestMay 6, 2022
5 tasks
@ezio-melotti
Copy link
MemberAuthor

Thanks everyone for the feedback and testing! I'll wait for the feedback of@JulienPalard before fixing the 2 failures and merging the PR.

@JulienPalard
Copy link
Member

@ezio-melotti Don't you prefer waiting forsphinx-contrib/sphinx-lint#27 to be merged, avoiding two commits on cpython doing the same thing?

It would look likesphinx-lint --enable default-role, which is more transparent than--severity 0 --disable 'line too long'.

ezio-melotti reacted with thumbs up emoji

Co-authored-by: Julien Palard <julien@palard.fr>
@ezio-melottiezio-melotti marked this pull request as ready for reviewMay 14, 2022 18:22
@ezio-melotti
Copy link
MemberAuthor

The PR should be ready to be merged.
@AlexWaygood /@zware: can you check if it works on Windows? (you might have to updatesphinx-lint)

@ezio-melotti
Copy link
MemberAuthor

Looks likeit works on WIndows too.

@miss-islington
Copy link
Contributor

Thanks@ezio-melotti for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks@ezio-melotti for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks@ezio-melotti for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry@ezio-melotti, I had trouble checking out the3.9 backport branch.
Please backport usingcherry_picker on command line.
cherry_picker 953ab0795243900ccccaaca069d932730a86fc20 3.9

@miss-islington
Copy link
Contributor

Sorry,@ezio-melotti, I could not cleanly backport this to3.10 due to a conflict.
Please backport usingcherry_picker on command line.
cherry_picker 953ab0795243900ccccaaca069d932730a86fc20 3.10

@miss-islington
Copy link
Contributor

Sorry@ezio-melotti, I had trouble checking out the3.11 backport branch.
Please backport usingcherry_picker on command line.
cherry_picker 953ab0795243900ccccaaca069d932730a86fc20 3.11

ezio-melotti added a commit to ezio-melotti/cpython that referenced this pull requestMay 15, 2022
* Restore default role check in `make check`.* Options first, then files.* Update `make.bat` too.* Add a comment explaining the extra options.* No reason to ignore the README.rst.* Enable default-role check in sphinx-lint.Co-authored-by: Julien Palard <julien@palard.fr>* Update sphinx-lint default-role check.* Fix use of the default role in the docs.* Update make.bat to check for the default role too.* Fix comment in make.bat.Co-authored-by: Julien Palard <julien@palard.fr>(cherry picked from commit953ab07)Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>
@bedevere-botbedevere-bot removed the needs backport to 3.11only security fixes labelMay 15, 2022
@bedevere-bot
Copy link

GH-92821 is a backport of this pull request to the3.11 branch.

ezio-melotti added a commit that referenced this pull requestMay 15, 2022
* Restore default role check in `make check`.* Options first, then files.* Update `make.bat` too.* Add a comment explaining the extra options.* No reason to ignore the README.rst.* Enable default-role check in sphinx-lint.Co-authored-by: Julien Palard <julien@palard.fr>* Update sphinx-lint default-role check.* Fix use of the default role in the docs.* Update make.bat to check for the default role too.* Fix comment in make.bat.Co-authored-by: Julien Palard <julien@palard.fr>(cherry picked from commit953ab07)Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>
@JulienPalard
Copy link
Member

Thanks@ezio-melotti!

ezio-melotti and hugovk reacted with hooray emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@vstinnervstinnervstinner left review comments

@JulienPalardJulienPalardJulienPalard left review comments

@zwarezwarezware left review comments

@AlexWaygoodAlexWaygoodAlexWaygood left review comments

@pgansslepgansslepganssle approved these changes

@ethanfurmanethanfurmanAwaiting requested review from ethanfurmanethanfurman is a code owner

@abalkinabalkinAwaiting requested review from abalkinabalkin is a code owner

Assignees

@ezio-melottiezio-melotti

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@ezio-melotti@vstinner@AlexWaygood@JulienPalard@miss-islington@bedevere-bot@pganssle@zware

[8]ページ先頭

©2009-2025 Movatter.jp