Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
Restore default role check in make 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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restore default role check in make check. #92290
Uh oh!
There was an error while loading. Please reload this page.
Conversation
ezio-melotti commented May 4, 2022
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'. |
Uh oh!
There was an error while loading. Please reload this page.
ezio-melotti commented May 4, 2022
I added the same changes to
In addition, bfba2cd added the |
vstinner commented May 5, 2022
The CI fails: |
ezio-melotti commented May 5, 2022
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 commented May 5, 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.
Tried the patch out on my Windows machine: (I'll be honest, I usually don't bother with the |
Uh oh!
There was an error while loading. Please reload this page.
ezio-melotti commented May 7, 2022
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 commented May 7, 2022
@ezio-melotti Don't you prefer waiting for sphinx-contrib/sphinx-lint#27 to be merged, avoiding two commits on cpython doing the same thing? It would look like |
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Julien Palard <julien@palard.fr>
ezio-melotti commented May 14, 2022
The PR should be ready to be merged. |
ezio-melotti commented May 14, 2022
Looks like it works on WIndows too. |
miss-islington commented May 15, 2022
Thanks @ezio-melotti for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9. |
miss-islington commented May 15, 2022
Thanks @ezio-melotti for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10. |
miss-islington commented May 15, 2022
Thanks @ezio-melotti for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11. |
miss-islington commented May 15, 2022
Sorry @ezio-melotti, I had trouble checking out the |
miss-islington commented May 15, 2022
Sorry, @ezio-melotti, I could not cleanly backport this to |
miss-islington commented May 15, 2022
Sorry @ezio-melotti, I had trouble checking out the |
* 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 commit 953ab07) Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>
bedevere-bot commented May 15, 2022
GH-92821 is a backport of this pull request to the 3.11 branch. |
* 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 commit 953ab07) Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>
JulienPalard commented May 15, 2022
Thanks @ezio-melotti! |
This PR restores the check for the default role (see GH-92289) by doing two things. First, it sets the
severityto0adding--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 fixes
Makefile. If this approach is sound,make.batshould be fixed too.Using
--enableinstead might be a better approach, once sphinx-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