FIX: FreeSurfer without adding some fs* to --output-spaces
#1643
Conversation
…spaces`` Tested locally, closes nipreps#1641.
Additionally, parse commit title to skip other build targets if it starts with ``docs:`` or ``docs(text):``. FORCING commit to make sure that CircleCI workflow has not been broken.
| - run: | ||
| name: Check whether build should be skipped | ||
| command: | | ||
| if [[ "$( git log --format='format:%s' -n 1 $CIRCLE_SHA1 | grep -i -E '^docs?(\(\w+\))?:' )" != "" ]]; then |
There was a problem hiding this comment.
I see you've created a new committing convention; is it documented somewhere?
Also, is it a good idea to skip tests if the most recent commit is a doc update? It seems likely to trip people up.
There was a problem hiding this comment.
is it documented somewhere?
It's not. I can add documentation if we finally decide to keep it.
It seems likely to trip people up.
Why? we should be careful though and always request for a full build before merging, but this adds nice flexibility when you are in the middle of a big PR (or situations like Franklin's).
There was a problem hiding this comment.
It seems likely to trip people up.
Why?
Just because if I were putting together a batch of commits to submit, docs would often be the last thing before submitting.
There was a problem hiding this comment.
For users: at that point we would walk them through forcing one more commit
For us, we can just use dox to indicate that the commit is documentation but you want to trigger a full build.
Honestly, I think the risks/value tradeoff is clear here. On the bright side, it is a lot easier to ask people to commit with docs: than [skip ds005][skip ds054][skip ds210][skip tests] which would be equivalent (although it would waste time building the docker image).
There was a problem hiding this comment.
Would you be okay merging and keeping the discussion (here or elsewhere)? I think the discussion is worth having and merging this would not be harmful.
Tested locally, closes #1641.