GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-86404: Doc: Drop now unused make suspicious and rstlint. by JulienPalard · Pull Request #98179 · python/cpython

Conversation

@JulienPalard

@JulienPalard JulienPalard commented Oct 11, 2022

Copy link
Copy Markdown
Member

They have been replaced by
sphinx-lint.

closes gh-86404.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hugovk reacted with hooray emoji
All reactions
  • 🎉 1 reaction
@bedevere-bot bedevere-bot added the awaiting core review label Oct 11, 2022
@JulienPalard
Doc: Drop now unused make suspicious and rstlint.
dc5cf61
They have been replaced by
[sphinx-lint](https://github.com/sphinx-contrib/sphinx-lint).

closes pythongh-86404.
@JulienPalard
JulienPalard force-pushed the mdk-drop-suspicious branch from 64192cb to dc5cf61 Compare October 11, 2022 07:42
@JulienPalard
JulienPalard requested review from AA-Turner and ezio-melotti October 11, 2022 07:44
ezio-melotti
ezio-melotti reviewed Oct 11, 2022
View reviewed changes

@ezio-melotti ezio-melotti left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
vstinner
vstinner approved these changes Oct 11, 2022
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Oct 11, 2022
@vstinner

vstinner commented Oct 11, 2022

Copy link
Copy Markdown
Member

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

  • I cannot find suspicious in https://github.com/python/release-tools/
  • I cannot find suspicious in https://peps.python.org/pep-0101/ neither

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!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JulienPalard

JulienPalard commented Oct 11, 2022

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JulienPalard

JulienPalard commented Oct 11, 2022

Copy link
Copy Markdown
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 and arhadthedev reacted with rocket emoji
All reactions
  • 🚀 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ezio-melotti

ezio-melotti commented Oct 11, 2022

Copy link
Copy Markdown
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

cpython/Doc/whatsnew/3.12.rst

Lines 212 to 223 in e0ae9dd

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JulienPalard

JulienPalard commented Oct 11, 2022

Copy link
Copy Markdown
Member Author

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

OK, so, why not :)

ezio-melotti reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JulienPalard
Adding whatsnew.
0f31d46
@JulienPalard

JulienPalard commented Oct 11, 2022

Copy link
Copy Markdown
Member Author

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

Done.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ezio-melotti
ezio-melotti approved these changes Oct 11, 2022
View reviewed changes

@ezio-melotti ezio-melotti left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
AA-Turner
AA-Turner approved these changes Oct 11, 2022
View reviewed changes

@AA-Turner AA-Turner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks @JulienPalard!

A

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@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
@bedevere-bot bedevere-bot removed the awaiting merge label Oct 11, 2022
@JulienPalard
JulienPalard deleted the mdk-drop-suspicious branch October 11, 2022 13:31
@AlexWaygood

AlexWaygood commented Oct 11, 2022

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Oct 11, 2022

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AlexWaygood

AlexWaygood commented Oct 11, 2022

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ezio-melotti

ezio-melotti commented Oct 12, 2022

Copy link
Copy Markdown
Member

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

  • bpo-42238: Doc CI: Disable suspicious checks. #26575

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 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AlexWaygood

AlexWaygood commented Oct 12, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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

  • bpo-42238: Doc CI: Disable suspicious checks. #26575

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!

ezio-melotti reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JulienPalard

JulienPalard commented Oct 12, 2022

Copy link
Copy Markdown
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).

AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

Reviewers

@vstinner vstinner vstinner approved these changes
@AA-Turner AA-Turner AA-Turner approved these changes
@ezio-melotti ezio-melotti ezio-melotti approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

docs Documentation in the Doc dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Deprecate suspicious.py?

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@JulienPalard @vstinner @ezio-melotti @AlexWaygood @AA-Turner @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.