Skip to content

[MRG] Enables CircleCI to fail when sphinx warns #15633

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 24 commits into from
Nov 19, 2019
Merged

[MRG] Enables CircleCI to fail when sphinx warns #15633

merged 24 commits into from
Nov 19, 2019

Conversation

thomasjpfan
Copy link
Member

@thomasjpfan thomasjpfan commented Nov 15, 2019 •
edited
Loading

Closes #6025

Enables CircleCI to fail when sphinx warns

cmarmo and others added 20 commits October 23, 2019 17:02
@cmarmo
Make circleCI exit with error if sphinx warnings are generated.
a642e32
@cmarmo
Merge branch 'master' into cierroronwarning
5ed1c9c
@cmarmo
Fix unsafe exit.
96cf750
@cmarmo
[doc build] Fix unsafe exit.
c19c4e3
@cmarmo
Look for example images in modified files.
4d07039
@cmarmo
Fix for no images in modified files.
0196e9e
@cmarmo
Merge branch 'master' into cierroronwarning
0a0cf5f
@cmarmo
Exclude build_doc.sh
c6735d9
@cmarmo
Merge branch 'master' into cierroronwarning
455f500
@cmarmo
Merge branch 'master' into cierroronwarning
6ac8be2
@cmarmo
Merge branch 'cierroronwarning' of https://github.com/cmarmo/scikit-l…
5c0bafc 
…earn into cierroronwarning
@cmarmo
Merge branch 'master' into cierroronwarning
7e664e0
@cmarmo
Generalized implementation. Fix concatenation issues.
a2f3255
@cmarmo
Merge branch 'master' into cierroronwarning
0749882
@cmarmo
Address comments
bc08415
@cmarmo
Add comments.
f91903e
@cmarmo
Merge branch 'master' into cierroronwarning
d533bcb
@cmarmo
Implement @thomasjpfan solution
2fef5e7
@thomasjpfan
CLN Uses warnings directly
fa3ca3f
@thomasjpfan
TST Introduces error
8924e17
@thomasjpfan thomasjpfan mentioned this pull request Nov 15, 2019
Closed
@thomasjpfan thomasjpfan changed the title [NOMRG] Testing circleci [MRG] Enables CircleCI to fail when sphinx warns Nov 16, 2019
@thomasjpfan
Copy link
Member Author

CC @ogrisel @adrinjalali @cmarmo

This is PR is mostly #15355 with a minor change to address #15355 (comment)

cmarmo
cmarmo approved these changes Nov 18, 2019
View reviewed changes
Copy link
Contributor

@cmarmo cmarmo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM to me: nice and clean.
Just to answer myself: thumb images are embedded in doc/auto_examples/index.rst so I'm still puzzled about the necessity of grepping them.
But this PR needs to be merged. :) Thanks

ogrisel
ogrisel approved these changes Nov 19, 2019
View reviewed changes
Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM, will merge. Thank you very much @cmarmo and @thomasjpfan.

@ogrisel ogrisel merged commit a723bea into scikit-learn:master Nov 19, 2019
@glemaitre
Copy link
Member

Uhm weird but I think that I am getting some false positive:

#13227
https://84125-843222-gh.circle-artifacts.com/0/doc/_changed.html

@cmarmo
Copy link
Contributor

cmarmo commented Nov 20, 2019

Apparently the thumb thing doesn't work. The EXAMPLES_PATTERN still contains the image name and not the script. @thomasjpfan may I help and change that? Or you have some time for do it yourself?

@glemaitre
Copy link
Member

sed -e 's/_([[:digit:]][[:digit:]][[:digit:]]|thumb).png/.py/')

is not working somehow

@glemaitre
Copy link
Member

Apparently the thumb thing doesn't work. The EXAMPLES_PATTERN still contains the image name and not the script. @thomasjpfan may I help and change that? Or you have some time for do it yourself?

@cmarmo quicker than me :) you might also be quicker to fix it. I let you make a PR and I will review and merge it quickly such that we don't have all the CIs complaining whenever an example is touched.

@thomasjpfan thomasjpfan mentioned this pull request Nov 20, 2019
Closed
@cmarmo cmarmo mentioned this pull request Nov 20, 2019
Merged
adrinjalali pushed a commit to adrinjalali/scikit-learn that referenced this pull request Nov 25, 2019
@thomasjpfan @adrinjalali
[MRG] Enables CircleCI to fail when sphinx warns (scikit-learn#15633)
07ca826
jnothman pushed a commit that referenced this pull request Nov 28, 2019
@thomasjpfan @jnothman
[MRG] Enables CircleCI to fail when sphinx warns (#15633)
2aef81a
panpiort8 pushed a commit to panpiort8/scikit-learn that referenced this pull request Mar 3, 2020
@thomasjpfan
[MRG] Enables CircleCI to fail when sphinx warns (scikit-learn#15633)
4849954
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogrisel ogrisel ogrisel approved these changes

@cmarmo cmarmo cmarmo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make sphinx build without warnings, let circle IO error on warnings
4 participants
@thomasjpfan @glemaitre @cmarmo @ogrisel