github: Run deps workflow against base branch and add branch name suffixes for easier debuggability #8010
Conversation
arjan-bal
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8010 +/- ##
==========================================
+ Coverage 81.93% 82.13% +0.19%
==========================================
Files 381 384 +3
Lines 38581 38750 +169
==========================================
+ Hits 31613 31826 +213
+ Misses 5640 5606 -34
+ Partials 1328 1318 -10 |
.github/workflows/deps.yml
Outdated
| BEFORE="$(mktemp -d --suffix="-${BEFORE_SUFFIX}")" | ||
| AFTER="$(mktemp -d --suffix="-${AFTER_SUFFIX}")" |
There was a problem hiding this comment.
Idea:
How about have one mktemp -d and then make two subdirectories of it which are the same as the existing BEFORE_SUFFIX and AFTER_SUFFIX and then cd to the mktemp dir before doing the diff so the relative path names are the names of the git refs? WDYT?
There was a problem hiding this comment.
That's a good idea. Changed accordingly.
There was a problem hiding this comment.
I changed the subdir names to before and after to keep things simple. It avoids having to sanitize the branch names to ensure they are valid linux paths, handling same source and target ref names and other possible issues.
arjan-bal
force-pushed
the
fix-deps-gh-action
branch
2 times, most recently
from
6a3e129 to
f6a4f58
Compare
arjan-bal
force-pushed
the
fix-deps-gh-action
branch
from
f6a4f58 to
e482d57
Compare
arjan-bal
force-pushed
the
fix-deps-gh-action
branch
from
68fece6 to
cf7cd4e
Compare
.github/workflows/deps.yml
Outdated
| @@ -30,14 +30,20 @@ jobs: | |||
| # Run the commands to generate dependencies before and after and compare. | |||
| - name: Compare dependencies | |||
| run: | | |||
| BEFORE="$(mktemp -d)" | |||
| AFTER="$(mktemp -d)" | |||
| set -eux | |||
There was a problem hiding this comment.
I would prefer not to have this (at least the -x (tracing) part) since the output should generally be pretty human readable, and the commands should be pretty stable.
The reason I did -x in the vet script was mainly laziness around not wanting to output everything the script was doing before & after it did it. (In hindsight that was probably a bad idea, as the output confuses most people.)
.github/workflows/deps.yml
Outdated
| # Run grep in a sub-shell since bash does not support ! in the middle of a pipe | ||
| diff -u0 -r "${BEFORE}" "${AFTER}" | bash -c '! grep -v "@@"' | ||
| diff -u0 -r "./before" "./after" | bash -c '! grep -v "@@"' |
There was a problem hiding this comment.
Nit: remove the ./ and I think the output will look a bit nicer
Running the test against the base branch (instead of master) ensures the tests don't fail unnecessarily for PRs raised against release branches.
Env vars available in Github actions: docs
RELEASE NOTES: N/A