Skip build test for non essentials #690 #699
Conversation
.github/workflows/build-and-test.yml
Outdated
| - name: "Check changes" | ||
| # TODO also include skipping build but allow docs to be processed | ||
| run: | | ||
| if git diff --exit-code --name-only origin/master . ':!.gitignore' ':!docs' ':!*.md' ':!*.txt' ':!*.yml'; then |
There was a problem hiding this comment.
Comparing to origin/master when on a PR will yield big diffs with older PRs.
Diff with common ancestor (see git merge-base) between PR branch and origin/master would be better IMHO. Here you can use the shortcut ...: git diff origin/master...HEAD...
There was a problem hiding this comment.
You are right. I wanted to make it more fitting for different branches and parents, but forgot to actually do it 😊
More something like git diff --exit-code HEAD^ HEAD
There was a problem hiding this comment.
I don't know Github at all. Maybe they are some variables to get the "base branch" of the PR ?
fc1451b to
40d40cd
Compare
.github/workflows/build-and-test.yml
Outdated
| @@ -20,7 +20,7 @@ jobs: | |||
| - name: "Check changes" | |||
| # TODO also include skipping build but allow docs to be processed | |||
| run: | | |||
| if git diff --exit-code --name-only origin/master . ':!.gitignore' ':!docs' ':!*.md' ':!*.txt' ':!*.yml'; then | |||
| if git diff --exit-code --name-only HEAD^ HEAD ':!.gitignore' ':!docs' ':!*.md' ':!*.txt' ':!*.yml'; then | |||
There was a problem hiding this comment.
That doesn't work on PR that contains multiple commits, but it's better than nothing :)
There was a problem hiding this comment.
But with that one you could use a determined value for depth, instead of 0
New commits breaks CoW tests
(Because the CoW tests are not idempotent and depends on the mount-point being used in tmpfs tests)
f6c6849 to
77dac8f
Compare
|
Euuh... I wasn't aware that me tinkering with Github actions on |
Echo diff result Fix
77dac8f to
4f956de
Compare
|
This |
As long as github actions/checkout is using "origin" as the remote name ;) |
.github/workflows/build-and-test.yml
Outdated
| - name: "Check need for testing" | ||
| # TODO also include skipping build but allow docs to be processed | ||
| run: | | ||
| if git diff --exit-code --name-only origin/$GITHUB_BASE_REF HEAD ':!.gitignore' ':!docs' ':!*.md' ':!*.txt'; then |
There was a problem hiding this comment.
what about origin/$GITHUB_BASE_REF...HEAD ?
There was a problem hiding this comment.
Not sure, tbh. I am accustomed to Jenkins and Bitbucket where I would get the merge result in the PR to test with. So this (the .. equivalent) should be fine. It seems GHA works differently which makes we wonder whether we should also have a check for the intended result before the actual merge is done. It seems we are only testing the branch, which I assume the coder already has tested as well, not bad to repeat but there is less value in that. Digging a bit deeper.
There was a problem hiding this comment.
GHA has GITHUB_SHA apparently as the merge commit. Going the check that one.
Yes 😇 I haven't found a variable which returns the full path. So it is a bit 🤞 |
Skip the build test step when no executable changes are made.
This will speed up the process when only documents and such are modified. Save on quota too.
This can be improved to more fine grained step, when so desired:
Adresses #690