[MRG + 1] CI make html-noplot when doc/ and examples/ not modified #7923
Conversation
cba8697 to
1c0c8d4
Compare
| @@ -2,18 +2,74 @@ | |||
| set -x | |||
| set -e | |||
|
|
|||
| # Introspect the commit to know whether or not we should skip building the | |||
| # Inspect the commit to know whether or not we should skip building the | |||
| # documentation: a pull request that does not change any file in doc/ or | |||
| # examples/ folder should be skipped unless the "[doc: build]" is found the | |||
There was a problem hiding this comment.
The comment should say [doc build] and not [doc: build]
There was a problem hiding this comment.
Also I believe the information in the check_build_doc.py docstring is a bit more complete, that would be a good idea to move it here.
| filenames=$(git diff --name-only $git_range) | ||
| if [ -z "$filenames" ] | ||
| then | ||
| echo QUICK BUILD: failed to get changed filenames for $git_range |
There was a problem hiding this comment.
Just curious, when can that happen, empty commit?
There was a problem hiding this comment.
Empty commit; or git error.
There was a problem hiding this comment.
Not that crucial but I believe a git error will stop the script since you have set -e at the beginning of the script.
| echo "$filenames" | ||
| } | ||
|
|
||
| touch ~/log.txt # the "test" segment needs this file |
There was a problem hiding this comment.
You can remove this since the test is now a dummy echo command and not cat log.txt | grep Traceback
Also test [doc build]
|
Thanks for the comments. Updated. |
| echo QUICK BUILD: failed to get changed filenames for $git_range | ||
| return | ||
| fi | ||
| if echo "$filenames" | grep -q -e ^examples/ -e ^doc/ |
There was a problem hiding this comment.
I got tripped up by this one a few times: since you have set -e a grep without a match will have an exit code 1 and stop the script.
I have to say I don't know what is the best way to handle this so I used things like this in flake8_diff.sh:
echo $filenames | grep bla || echo no_match
There was a problem hiding this comment.
surely that's not what happens. set -e doesn't affect test in an if context; and it doesn't affect pipes...??
There was a problem hiding this comment.
Ah you are right, there is no problem within the if context.
| echo QUICK BUILD: failed to inspect commit $CIRCLE_SHA1 | ||
| return | ||
| fi | ||
| if [[ "$commit_msg" =~ '[doc skip]' ]] |
There was a problem hiding this comment.
I don't actually get why this works. Why is the RHS not treated as a character class?
There was a problem hiding this comment.
Good catch, I think the RHS is treated as a character class so you need to escape the square brackets:
if [[ "d" =~ '[doc skip]' ]]; then echo yes; else echo no; fi # outputs yes
There was a problem hiding this comment.
I'm not getting yes output for that on two different OSes. Maybe this is why it should be written in Python
There was a problem hiding this comment.
Oh God. It's Bash string madness again. Apparently with the ' it's processed as an exact match...
There was a problem hiding this comment.
I'm not getting yes output for that on two different OSes. Maybe this is why it should be written in Python
I was using a zsh shell, naively assuming that the behaviour would be the same. As you said in bash, 'a' =~ [abcd] matches but not 'a' =~ '[abcd]'.
|
LGTM, the Travis failure is unrelated: |
86ebf25 to
a3d0baf
Compare
|
Note that skip just worked |
Great, thanks for testing it. |
| echo QUICK BUILD: no changed filenames for $git_range | ||
| return | ||
| fi | ||
| if echo "$filenames" | grep -q -e ^examples/ -e ^doc/ |
There was a problem hiding this comment.
I might be a bit slow. why do we need a full build for changes to doc?
There was a problem hiding this comment.
Well, historically we have. I don't mind much either way. Narrative docs tend to include images from examples, although most changes to docs don't change that, except where examples is also being modified.
|
LGTM apart from comment. I only checked the logic, not the syntax, I can't read shell well enough... |
|
Will merge. Happy to change so that |
Fixes #7815. By default, Circle CI will run
make html-noplot, withmake htmlwhendoc/orexamples/is touched.I've also merged
check_build_docintobuild_doc.sh, but this can be reverted.This is untested!