◐ Shell
reader mode source ↗
Skip to content

[workflow] Use Sphinx problem matcher on docs builds#20325

Merged
pablogsal merged 1 commit into
python:masterfrom
ammaraskar:sphix_problem_matcher
May 25, 2020
Merged

[workflow] Use Sphinx problem matcher on docs builds#20325
pablogsal merged 1 commit into
python:masterfrom
ammaraskar:sphix_problem_matcher

Conversation

@ammaraskar

@ammaraskar ammaraskar commented May 22, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

See https://discuss.python.org/t/using-github-problem-matchers-to-catch-warnings-early/4254 for more details.

This adds a problem matcher to show errors from the Sphinx build prominently in pull requests.

@ammaraskar ammaraskar closed this May 22, 2020
@ammaraskar ammaraskar reopened this May 22, 2020
@ammaraskar ammaraskar force-pushed the sphix_problem_matcher branch 3 times, most recently from 7172d99 to 52d771d Compare May 23, 2020 00:02
@ammaraskar ammaraskar force-pushed the sphix_problem_matcher branch from 52d771d to 819f2e7 Compare May 24, 2020 22:48
@ammaraskar ammaraskar requested review from pablogsal and zware May 24, 2020 22:57
@pablogsal

pablogsal commented May 24, 2020

Copy link
Copy Markdown
Member

@ammaraskar Could you simulate some sphinx problem to see how it looks? (You can then add another commit to revert it).

@ammaraskar ammaraskar force-pushed the sphix_problem_matcher branch 2 times, most recently from dea7559 to c550385 Compare May 24, 2020 23:47
@ammaraskar

ammaraskar commented May 24, 2020

Copy link
Copy Markdown
Member Author

I added two quick errors for testing, looks like -W has a fail-fast mechanism whereby the first warning causes the build to bail out so we don't see an error for the overline-underline mismatch.

@pablogsal

pablogsal commented May 25, 2020

Copy link
Copy Markdown
Member

Hmmm, giving that we are treating warnings as errors already, does it make sense to add the sphynx one? I find a bit misleading also that it only reports the first one (I understand why it happens), but if a contributor uses this new system to locate errors it would be very frustrating. Maybe I am missing some detail, though. What do you think?

@ammaraskar ammaraskar force-pushed the sphix_problem_matcher branch from c550385 to 5c44836 Compare May 25, 2020 03:42
@ammaraskar

ammaraskar commented May 25, 2020

Copy link
Copy Markdown
Member Author

Hmmm, giving that we are treating warnings as errors already, does it make sense to add the sphynx one?

Yeah, I was thinking this one is not as necessary as the compiler warning ones. Take a look now, I discovered there was --keep-going option along with Sphinx's -W that causes it to not fail-fast.

The main advantage of this would be that contributors who aren't that used to Sphinx/rst don't have to go looking through the logs to find out why the build is failing. It provides the errors in a more seamless way for a Github web editor workflow.

@pablogsal

pablogsal commented May 25, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

The main advantage of this would be that contributors who aren't that used to Sphinx/rst don't have to go looking through the logs to find out why the build is failing. It provides the errors in a more seamless way for a Github web editor workflow.

Yeah, that is true. I think there is some value in this.

Could you remove the failure changes so we can land this?

@ammaraskar ammaraskar force-pushed the sphix_problem_matcher branch from 5c44836 to bce56fd Compare May 25, 2020 20:53
@ammaraskar

ammaraskar commented May 25, 2020

Copy link
Copy Markdown
Member Author

Done :)

@pablogsal pablogsal merged commit 2602d97 into python:master May 25, 2020
@pablogsal

pablogsal commented May 25, 2020

Copy link
Copy Markdown
Member

🚀 Thanks for the good work @ammaraskar !

@ammaraskar ammaraskar mentioned this pull request Jun 2, 2020
Closed
zware pushed a commit that referenced this pull request Dec 28, 2020
@ammaraskar
[workflow] Use MSVC problem matcher for Windows action build (GH-18532) …
1031f23 
This makes warnings and errors from the compiler very prominent so this should help prevent warnings from sneaking into the code base and catch them in review. See https://discuss.python.org/t/using-github-problem-matchers-to-catch-warnings-early/4254 for more details

You can see a demo of this in action here: https://github.com/ammaraskar/cpython/pull/15/files#diff-9ba2eeca0f254ece0a9df4d7cb68e870

GCC and Sphinx matchers have previously been added in GH-18567 and GH-20325, respectively.
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@ammaraskar @adorilson
[workflow] Use MSVC problem matcher for Windows action build (pythonG… …
7638e83 
…H-18532)

This makes warnings and errors from the compiler very prominent so this should help prevent warnings from sneaking into the code base and catch them in review. See https://discuss.python.org/t/using-github-problem-matchers-to-catch-warnings-early/4254 for more details

You can see a demo of this in action here: https://github.com/ammaraskar/cpython/pull/15/files#diff-9ba2eeca0f254ece0a9df4d7cb68e870

GCC and Sphinx matchers have previously been added in pythonGH-18567 and pythonGH-20325, respectively.
@hugovk hugovk mentioned this pull request Jun 12, 2022
Merged
@hugovk hugovk mentioned this pull request Aug 16, 2023
Merged
@m-aciek m-aciek mentioned this pull request Jan 29, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pablogsal pablogsal pablogsal approved these changes

@zware zware Awaiting requested review from zware

Assignees

No one assigned

Labels

skip issue skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ammaraskar @pablogsal @the-knights-who-say-ni @bedevere-bot