-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
Restore default role check in make check
.
#92290
Conversation
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'. |
I added the same changes to
In addition, bfba2cd added the |
The CI fails:
|
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. |
Tried the patch out on my Windows machine:
(I'll be honest, I usually don't bother with the |
Thanks everyone for the feedback and testing! I'll wait for the feedback of @JulienPalard before fixing the 2 failures and merging the PR. |
@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 |
Co-authored-by: Julien Palard <julien@palard.fr>
The PR should be ready to be merged. |
Looks like it works on WIndows too. |
Thanks @ezio-melotti for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9. |
Thanks @ezio-melotti for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10. |
Thanks @ezio-melotti for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11. |
Sorry @ezio-melotti, I had trouble checking out the |
Sorry, @ezio-melotti, I could not cleanly backport this to |
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>
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>
Thanks @ezio-melotti! |
This PR restores the check for the default role (see GH-92289) by doing two things. First, it sets the
severity
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 fixes
Makefile
. If this approach is sound,make.bat
should be fixed too.Using
--enable
instead 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