-
Notifications
You must be signed in to change notification settings - Fork 53
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.
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
Fix racy workflows #1996
Fix racy workflows #1996
Conversation
I guess we can simply verify that by checking the toolkit commit matches the branch head. @frelon I am correct? |
The merge_commit_sha seems to not always be calculated correctly, try to use github.event.pull_request.head.sha instead. Signed-off-by: Fredrik Lönnegren <[email protected]>
You mean the GIT_COMMIT used in the Makefile? Yep, that would be one way to do it. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1996 +/- ##
=======================================
Coverage 72.53% 72.53%
=======================================
Files 76 76
Lines 8910 8910
=======================================
Hits 6463 6463
Misses 1913 1913
Partials 534 534 ☔ View full report in Codecov by Sentry. |
MMm there is still something weird in here. See the difference between the Build CLI and build-toolkit jobs. For unit tests:
e9a93d1 the current head commit of this branch. and for build-toolkit job:
Where 6af6e0f is the previous commit!! 😱 |
Yea, since the pull_request_target is used we will only see the new usage of github.event.pull_request.head.sha when this PR is merged. |
Oh I see, yes lets give it a try. |
The merge_commit_sha seems to not always be calculated correctly, try to use github.event.pull_request.head.sha instead.