Repository navigation
Specify target to local tag SHA in release create - #5104
Conversation
mislav
left a comment
mislav
left a comment
There was a problem hiding this comment.
I like this approach! Thank you
|
If I'm understanding this change correctly, this will set the target to the SHA of the tag, not the tag itself, right? I'm concerned that this will trigger the API to create a new tag with the same name, losing any tag annotations or GPG signatures that were created locally in the process.
Though I admit that I don't know how common it is for a tag to be signed or annotated, so I'm unsure if my experience is common enough to be a concern. Just my 2 cents, thank you to everyone for your hard work maintaining |
Hmm that's a good point about tag name vs. commit SHA; thank you @spenserblack. @samcoe Do you think we could change to that approach to avoid potentially recreating the unpushed tag on the remote? |
4122272 to
d296d7e
Compare
|
@mislav Ok I changed the approach. This one is definitely a bit more complicated and adds more complexity to the already complex
|
|
|
||
| var tagDescription string | ||
| if opts.RepoOverride == "" { | ||
| tagDescription, _ = gitTagInfo(opts.TagName) |
There was a problem hiding this comment.
I am using tagDescription as an indicator that the local tag exists. From my testing that has worked out fine, but are there any scenarios where a tag exists locally and the tag description is blank?
There was a problem hiding this comment.
gitTagInfo should exit with an error status if the tag was not found. Perhaps you can use the lack of an error return value as an indicator that the tag was found?
There was a problem hiding this comment.
Hmm I tried this locally and git returned a 0 status when a tag didn't exist.
mislav
left a comment
mislav
left a comment
There was a problem hiding this comment.
Thanks for your hard work and doing all the iterations of this implementation! Only nitpicks remain 🙌
|
|
||
| var tagDescription string | ||
| if opts.RepoOverride == "" { | ||
| tagDescription, _ = gitTagInfo(opts.TagName) |
There was a problem hiding this comment.
gitTagInfo should exit with an error status if the tag was not found. Perhaps you can use the lack of an error return value as an indicator that the tag was found?
4 participants
This PR sets the
target_commitishparameter to the local tag SHA when present for therelease createcommand. This prevents the API from creating a new tag pointed at the incorrect SHA if the local tag has not been pushed.cc #4571 (review)
Supersedes #4571
Closes #4357