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 — bpo-42238: [doc] moving from rstlint.py to sphinx-lint. by JulienPalard · Pull Request #31097 · python/cpython

Conversation

@JulienPalard

@JulienPalard JulienPalard commented Feb 3, 2022 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

https://bugs.python.org/issue42238

Sorry, something went wrong.

Uh oh!

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

All reactions
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Feb 3, 2022
@bedevere-bot bedevere-bot added the awaiting core review label Feb 3, 2022
@arhadthedev

arhadthedev commented Feb 3, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

This PR touches a bunch of Misc/NEWS.d/next/Build/*.rst changing nothing visible (except one insertion of a missing colon). Is it intended? Looks like spurious CR/LF conversion slipped in.

All reactions

Sorry, something went wrong.

Uh oh!

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

@arhadthedev arhadthedev mentioned this pull request Feb 3, 2022
Freeze a README link to rstlint.py sphinx-contrib/sphinx-lint#6
Merged
jeanas
jeanas reviewed Feb 3, 2022
View reviewed changes
Comment thread Doc/requirements.txt Outdated

@jeanas jeanas Feb 3, 2022

Copy link
Copy Markdown
Contributor

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.

Shouldn't this dependency be pinned? I say this because if we refactor sphinx-lint (e.g. as a Sphinx builder as discussed in sphinx-contrib/sphinx-lint#1), it will require adaptation here. A new release will have to be made with the changes before they can be used in CPython, and between the release and the actual switch, things will be broken (plus people might not understand what's going on it if they don't pull recent changes but install a newer sphinx-lint).

Sorry, something went wrong.

Uh oh!

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

All reactions

@jeanas jeanas Feb 3, 2022

Copy link
Copy Markdown
Contributor

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.

Another thought: it will make it easier to add checks that might have a few false positives (preferably there won't be any, but it could be tricky to eliminate some rare cases).

Sorry, something went wrong.

Uh oh!

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

All reactions

@JulienPalard JulienPalard Feb 3, 2022

Copy link
Copy Markdown
Member Author

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.

it will require adaptation here.

I don't think so, but I may miss something. sphinx-lint should be able to find the conf.py by itself, and we'll be able to add a sphinx install dependency, so it looks like it should be doable without adaptation.

Still we can pin <1 and use semver, so if we break something we can bump the major version if you prefer.

Sorry, something went wrong.

Uh oh!

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

All reactions

@jeanas jeanas Feb 3, 2022

Copy link
Copy Markdown
Contributor

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.

The normal way to invoke a Sphinx builder would basically be sphinx-build -b lint. I haven't investigated if it's feasible to allow direct invocation by a script; there would be implications on which Sphinx to use for example (from some venv or not?). I think it can't hurt to require <1.

Sorry, something went wrong.

Uh oh!

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

All reactions

@JulienPalard JulienPalard Feb 3, 2022

Copy link
Copy Markdown
Member Author

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.

OK for < 1, done.

Sorry, something went wrong.

Uh oh!

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

All reactions
@JulienPalard
JulienPalard force-pushed the mdk-sphinx-lint branch from 07ba286 to 72f8db8 Compare February 3, 2022 11:57
@JulienPalard

JulienPalard commented Feb 3, 2022

Copy link
Copy Markdown
Member Author

This PR touches a bunch of Misc/NEWS.d/next/Build/*.rst changing nothing visible (except one insertion of a missing colon). Is it intended? Looks like spurious CR/LF conversion slipped in.

This is intended: I edited those manually, that's because while moving rstlint.py to sphinx-lint I also worked on it for a few month and added a few other checks. The "invisible" issue was missing newlines at end of file, it was only found in 17 rst files (over 103 files in NEWS.d/next/).

No CR/LF issues found.

arhadthedev 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 pushed a commit to sphinx-contrib/sphinx-lint that referenced this pull request Feb 3, 2022
@arhadthedev
Freeze a README link to rstlint.py (#6)
232b815
README.md links to rstlint.py in a dynamic head of `main` CPython
branch:

```
    https://github.com/python/cpython/blob/main/Doc/tools/rstlint.py
                                           ^^^^
```

However, after python/cpython#31097 is merged,
the link will stop working. To keep the link working, `main` needs to be
replaced with some concrete commit SHA.
@JulienPalard

JulienPalard commented Feb 7, 2022

Copy link
Copy Markdown
Member Author

I « realized » that even if the script is hidden under Doc/tools, it exist, and as so it is used by someone somewhere, so instead of removing the file I just added a warning to it.

All reactions

Sorry, something went wrong.

Uh oh!

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

JulienPalard added 5 commits February 7, 2022 11:05
@JulienPalard
bpo-42238: [doc] moving from rstlint.py to sphinx-lint.
ffb8f98
@JulienPalard
Add news.
22da852
@JulienPalard
Pin a bit, just in case we want to release breaking changes.
a9bc65f
@JulienPalard
Don't drop the file that soon, notify about its move instead.
ee7f4d9
@JulienPalard
FIX: Missing newline at end of file.
9b02019
@JulienPalard
JulienPalard force-pushed the mdk-sphinx-lint branch from c9d51e6 to 9b02019 Compare February 7, 2022 10:06
@JulienPalard
JulienPalard merged commit b878b3a into python:main Feb 10, 2022
@bedevere-bot bedevere-bot removed the awaiting core review label Feb 10, 2022
@JulienPalard
JulienPalard deleted the mdk-sphinx-lint branch February 10, 2022 07:59
sobolevn added a commit to sobolevn/cpython that referenced this pull request Feb 10, 2022
@sobolevn
Fix sphinx-lint after python#31097 and b878b3a
1419399
@sobolevn sobolevn mentioned this pull request Feb 10, 2022
Fix sphinx-lint after #31097 and b878b3a #31248
Merged
miss-islington pushed a commit that referenced this pull request Feb 10, 2022
@sobolevn
Fix sphinx-lint after #31097 and b878b3a (GH-31248)
dee020a
`main` branch is failing, see https://dev.azure.com/python/cpython/_build/results?buildId=96616&view=logs&j=4db1505a-29e5-5cc0-240b-53a8a2681f75&t=a975920c-8356-5388-147c-613d5fab0171

Logs:

```
PATH=./venv/bin:$PATH sphinx-lint -i tools -i ./venv -i README.rst
No problems found.
PATH=./venv/bin:$PATH sphinx-lint ../Misc/NEWS.d/next/
[1] ../Misc/NEWS.d/next/Library/2022-02-09-00-53-23.[bpo-45863]().zqQXVv.rst:0: No newline at end of file (no-newline-at-end-of-file).
[1] ../Misc/NEWS.d/next/Build/2022-01-19-11-08-32.[bpo-46430]().k403m_.rst:0: No newline at end of file (no-newline-at-end-of-file).
2 problems with severity 1 found.
```

This PR fixes these two problems, so `main` is green again.

Related PR: #31097
CC @JulienPalard

Automerge-Triggered-By: GH:JulienPalard
@sweeneyde sweeneyde mentioned this pull request Feb 10, 2022
Include Newline at end of NEWS entry python/blurb_it#256
Closed
@JulienPalard JulienPalard mentioned this pull request Apr 10, 2022
Deprecate suspicious.py? #86404
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@jeanas jeanas jeanas left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

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

5 participants

@JulienPalard @arhadthedev @jeanas @the-knights-who-say-ni @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.