-
Notifications
You must be signed in to change notification settings - Fork 42
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: update merge commit command #375
Conversation
cf632f9
to
8278c9a
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #375 +/- ##
==========================================
+ Coverage 96.01% 96.03% +0.01%
==========================================
Files 81 81
Lines 2814 2824 +10
==========================================
+ Hits 2702 2712 +10
Misses 112 112
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
8278c9a
to
993eac0
Compare
993eac0
to
824c68c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix seems fine, but applying it only to GHActions adapter will not actually solve the problem described in #372 .
That problem affects mostly unknown CI providers (that are considered "local").
I suggest moving the fix logic to a helper function, possibly in codecov_cli.helpers.git.py
and apply this behavior to all CI adapters by possibly correcting the commit suggested here
There doesn't seem to be a need to change the GHActions adapter at this time.
return parents_hash[1] | ||
merge_commit_regex = r"^[a-z0-9]{40} [a-z0-9]{40}$" | ||
merge_commit_message = subprocess.run( | ||
["git", "show", "--no-patch", "--format=%P"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't actually have to change this adapter's logic. The command being used accounts for this fact and already "fixes" the merge commit if that's the case.
$ git rev-parse HEAD^@
1b00e154fb8a8a7ad09da4f9c6c95e70c5fa1565
211b95514359270c24c3e4e2058b3eef83db75e4 <-- this would be returned by the old logic
$ git show --no-patch --format=%P | cat
1b00e154fb8a8a7ad09da4f9c6c95e70c5fa1565 211b95514359270c24c3e4e2058b3eef83db75e4
I'm closing this PR. I moved his logic to #397 instead (but using the commend from github actions) It aims to be the same thing, but in a different part of the code. |
fixes #372
This PR mirrors what is done in the uploader and bash uploader (what a throwback) to collect the proper commit for merge commits.