Skip to content
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

gh-86404: Doc: Drop now unused make suspicious and rstlint. #98179

Merged
merged 2 commits into from
Oct 11, 2022

Conversation

JulienPalard
Copy link
Member

They have been replaced by
sphinx-lint.

closes gh-86404.

Copy link
Member

@ezio-melotti ezio-melotti left a comment

Choose a reason for hiding this comment

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

LGTM.
Does this need an entry in the 3.12 whatsnew?

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM. sphinx-lint is a great success!

It's good that Doc/tools/susp-ignored.csv can be removed. It can be removed because sphinx-lint has less false positives, right?

@vstinner
Copy link
Member

It seems like make suspicious is no longer used by Python releases: good.

cc @pablogsal @Yhg1s


sphinx-lint is run by the "Docs / Docs" CI job on this PR: "Check documentation" step:

PATH=./venv/bin:$PATH sphinx-lint --enable default-role ../Misc/NEWS.d/next/

In short, sphinx-lint is now run on all documentation changes, not only late during releases. Issues are catched earlier: good!

@JulienPalard
Copy link
Member Author

Does this need an entry in the 3.12 whatsnew?

I don't think so as it's not a new thing, it's an old thing being cleared (it was a tool to help migration from LaTeX documentation to Sphinx documentation, so its removal is long overdue).

It's been one year that the CI no longer uses make suspicious, I bet everybody just forgot about it existence. And the few humans running make suspicious manually are getting a deprecated message since November 2021.

@JulienPalard
Copy link
Member Author

It's good that Doc/tools/susp-ignored.csv can be removed. It can be removed because sphinx-lint has less false positives, right?

sphinx-lint has 0 false positives on a growing list of watched repositories, as of today:

https://github.com/jazzband/django-oauth-toolkit
https://github.com/neo4j/neo4j-python-driver
https://github.com/pandas-dev/pandas
https://github.com/python/peps
https://github.com/python/cpython
https://github.com/python/devguide/
https://github.com/spyder-ide/spyder-docs
https://github.com/sympy/sympy
https://github.com/sphinx-doc/sphinx

This list of repositories are automatically checked by sphinx-lint CI to "guarantee" no false positive are introduced as release time, like it happened a few days ago with the "default-role debacle" (which happened because this checker is not enabled by default, to it went under the radar, it's fixed now, we test each repos with their real set of checkers).

The goal is to stick to 0 false positive (aiming at the impossible... because why not).

@ezio-melotti
Copy link
Member

I don't think so as it's not a new thing, it's an old thing being cleared (it was a tool to help migration from LaTeX documentation to Sphinx documentation, so its removal is long overdue).

The whatsnew also has a section for deprecated/removed APIs/modules/tools.

See for example

Demos and Tools
===============
* Remove the ``Tools/demo/`` directory which contained old demo scripts. A copy
can be found in the `old-demos project
<https://github.com/gvanrossum/old-demos>`_.
(Contributed by Victor Stinner in :gh:`97681`.)
* Remove outdated example scripts of the ``Tools/scripts/`` directory.
A copy can be found in the `old-demos project
<https://github.com/gvanrossum/old-demos>`_.
(Contributed by Victor Stinner in :gh:`97669`.)

@JulienPalard
Copy link
Member Author

The whatsnew also has a section for deprecated/removed APIs/modules/tools.

OK, so, why not :)

@JulienPalard
Copy link
Member Author

The whatsnew also has a section for deprecated/removed APIs/modules/tools.

Done.

Copy link
Member

@ezio-melotti ezio-melotti left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@AA-Turner AA-Turner left a comment

Choose a reason for hiding this comment

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

Thanks @JulienPalard!

A

@AA-Turner AA-Turner added the docs Documentation in the Doc dir label Oct 11, 2022
@JulienPalard JulienPalard merged commit 4067c6d into python:main Oct 11, 2022
@JulienPalard JulienPalard deleted the mdk-drop-suspicious branch October 11, 2022 13:31
@AlexWaygood
Copy link
Member

@JulienPalard, looks like the 3.11 and 3.10 branches still run make suspicious -- should this PR be backported?

@vstinner
Copy link
Member

@JulienPalard, looks like the 3.11 and 3.10 branches still run make suspicious -- should this PR be backported?

I don't think that it's a good idea to backport. It's better to leave these stable branches as they are.

@AlexWaygood
Copy link
Member

That means that we will continue to get make suspicious false-positive errors on those branches, and we will continue to have to update Doc/tools/susp-ignored.csv for those branches when we get those false positives.

@ezio-melotti
Copy link
Member

3.11 isn't running it anymore, since it got removed here:

3.10 however is still running it. If this is a problem we could just switch to sphinx-lint in 3.10 too, but the removal of make suspicious and rstlint.py shouldn't be backported.

@AlexWaygood
Copy link
Member

AlexWaygood commented Oct 12, 2022

3.11 isn't running it anymore, since it got removed here:

3.10 however is still running it. If this is a problem we could just switch to sphinx-lint in 3.10 too, but the removal of make suspicious and rstlint.py shouldn't be backported.

Thanks, that makes sense! I think I remember seeing some false positives recently on some backports — they must have been backports to 3.10.

Agreed that backports should also be as minimal as possible!

@JulienPalard
Copy link
Member Author

I propose just disabling make suspicious in 3.10 in #98221.

I don't feel the need to add sphinx-lint there: the PR will come from main, where they have already been checked, and as victor says, better not touch those stable branches too much (I mean let's remove pain, not add some).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate suspicious.py?
6 participants