Repository navigation
Conversation
|
Checked behavior as asked in #4357 (comment). Looks like this does push the tag if it exists locally. Remote tag has message and signature. |
|
@vilmibm sorry for ping Nate, do you have any updates about this? :) |
samcoe
left a comment
There was a problem hiding this comment.
@guerinoni This is coming along nicely. I left a couple comments, nothing major though. Additionally, it would be great to see some tests around this functionality.
mislav
left a comment
There was a problem hiding this comment.
Apologies for jumping on this thread so late down the line, but I'd like to propose an alternate solution for the original problem, which was:
When using gh release create with a tag that exists in the local repository but not in origin, it creates a new tag instead of pushing the local tag. This seemed like confusing behavior to me.
I agree that it can be confusing that release create would ignore the local tag and create a new one from the state of the main branch. However, I do not think that auto-pushing the local tag is a great solution either since that approach raises new problems:
- GitHub CLI generally avoids doing surprising operations under the hood.
gh release createdoing an implicitgit push, while potentially helpful, can definitely be surprising to some. - How would someone intentionally ignore the local git tag (which may have been created for testing) in favor of generating a new git tag in the remote repository?
- How would someone skip auto-pushing that happens by default? We would have to potentially add a new flag to control that, which complicates the whole feature.
I propose a slightly different approach: to let the release create command fail if it references a tag that exists locally but not on the remote. An implementation of this could be: if a local tag exists and --target was not given, use the local tag name as the value for --target, which should fail if the tag wasn't pushed. Then, the user could opt out of this behavior by explicitly setting --target. How does that sound?
I didn't think about that in my initial issue, but that would resolve the issue, too 👍. Overall, the issue is not that |
|
Thanks for feedback... If this is useful for |
Please do; you can reuse this PR too (feel free to force-push to it if needed).
We're sorry for letting some PR sit open for a long time, but now we're committed to seeing this one through to completion! |
Fixes: #4357 Signed-off-by: Federico Guerinoni <[email protected]>
|
@guerinoni Thanks for the time and effort on this PR. Unfortunately, I think we are going to go a different direction with the implementation and more closely follow the path outlined here. I opened #5104 that moves in that direction. |
Fixes: #4357
Signed-off-by: Federico Guerinoni [email protected]