DOC: Post warnings as reviews on PRs#24731
Merged
tacaswell merged 3 commits intomatplotlib:mainfrom Dec 15, 2022
Merged
Conversation
story645
previously requested changes
Dec 15, 2022
| echo "No deprecation warnings in gallery") | ||
| (grep DeprecationWarning -r -l doc/build/html/tutorials || | ||
| echo "No deprecation warnings in tutorials") | ||
| (grep -rl DeprecationWarning doc/build/html/gallery || |
Member
There was a problem hiding this comment.
is it worth it to add the plot_types gallery or nah 'cause it's super specific?
Member
Author
There was a problem hiding this comment.
I just copied the existing warning checks, but I don't see any reason not to check plot_types as well.
didn't mean to press request changes button
This will fail until the doc build is complete, which causes unwanted notifications about the job 'failing'.
6fc3088 to
9954248
Compare
oscargus
approved these changes
Dec 15, 2022
Member
oscargus
left a comment
There was a problem hiding this comment.
Looks good! (Although I prefer not to have to rewrite the reviewdog call...)
(I'm not sure I'm a big believer in Review Dog doing a better job* than just posting a comment, but this works nicely and fits the current flow.
- As in new contributors missing what is the actual problem etc.)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
PR Summary
This posts the warnings and deprecations from the doc builds as artifacts, then fetches them from the GitHub Action for CircleCI posting, and posts them as commit checks.
It also removes the URL check that I added in #24579. While it can be fixed, it would still "error" when the build initially starts (because there are no artifacts) and we've lasted without it so far, so I took it out.
Now, I've found out that the
statusevent only triggers for the workflow on the default branch, so the changes here won't be directly reflected until merged (and is why the URL check thing above wasn't noticed before merging). Therefore, I have pushed this change to my fork'smainbranch, then opened a PR that breaks a reST target and a deprecation in an example. You can see this posting here: https://github.com/QuLogic/matplotlib/pull/25/filesFixes #22176
PR Checklist
Documentation and Tests
pytestpasses)Release Notes
.. versionadded::directive in the docstring and documented indoc/users/next_whats_new/.. versionchanged::directive in the docstring and documented indoc/api/next_api_changes/next_whats_new/README.rstornext_api_changes/README.rst