Skip to content

Add --verify-tag flag for release creation command - #6632

Merged
mislav merged 3 commits into
cli:trunkfrom
luanzeba:gh_release_verify_tag
Nov 28, 2022
Merged

mislav merged 3 commits into
cli:trunkfrom
luanzeba:gh_release_verify_tag

Conversation

@luanzeba

@luanzeba luanzeba commented Nov 16, 2022 •

Copy link
Copy Markdown
Contributor

Fixes #6566

When running gh release create <tag> --verify-tag, we query among repository tags via the GitHub API before creating the release, and abort the command if the tag was not found.

Things I'm not sure about

  • Is the test coverage good? Should I cover more cases with different flag combinations?
  • When checking for existence of remote tag in the "verify-tag" code path, we store the result in a variable to avoid the same API call later on. Is that reasonable?

Disclaimer

I'm very new to Go and contributing to the cli project, so I welcome nit picky comments in the PR review as a learning opportunity.

Fixes cli#6566

When running `gh release create <tag> --verify-tag`, we query <tag>
among repository tags via the GitHub API before creating the release,
and abort the command if the tag was not found.
Comment thread pkg/cmd/release/create/create.go Outdated
@luanzeba
luanzeba marked this pull request as ready for review November 16, 2022 19:05
@luanzeba
luanzeba requested a review from a team as a code owner November 16, 2022 19:05
@luanzeba
luanzeba requested review from mislav and removed request for a team November 16, 2022 19:05
@cliAutomation cliAutomation added the external pull request originating outside of the CLI core team label Nov 16, 2022

@mislav mislav left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty good! Thanks

Comment thread pkg/cmd/release/create/create.go Outdated
Comment thread pkg/cmd/release/create/create.go Outdated
Comment thread pkg/cmd/release/create/create.go Outdated
}
}

var remoteTagPresent bool

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable can now be pushed into the following if block as its scope because it won't be needed elsewhere

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in fad72b7

luanzeba and others added 2 commits November 16, 2022 15:09
@luanzeba
luanzeba force-pushed the gh_release_verify_tag branch from 1f9b64d to fad72b7 Compare November 16, 2022 20:12
@luanzeba
luanzeba requested a review from mislav November 16, 2022 20:22
@luanzeba

Copy link
Copy Markdown
Contributor Author

Thanks for the quick review @mislav ! This is ready for another pass when you have a chance. Also let me know if you'd like me to squash the commits

@mislav mislav left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@mislav
mislav merged commit 3017168 into cli:trunk Nov 28, 2022
@luanzeba
luanzeba deleted the gh_release_verify_tag branch November 28, 2022 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external pull request originating outside of the CLI core team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gh release create: Add flag to prevent a release if tag does not exist

3 participants