Skip to content
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

Support creating repos if not exist #31

Merged

Conversation

oliversun9
Copy link
Contributor

@oliversun9 oliversun9 commented May 31, 2023

Add optional key create_visibility to action, specifying it adds --create --create-visibility to buf generate invocation.

Note that CLI requires both --create and --create-visibility, whereas this action only requires create_visibility.

I prefer it this way because GitHub action error feedback loop is longer than the CLI. In an action, a user modifies the workflow file, commits it, pushes, waits for the action to run, checks result (a few clicks), sees an error. On the CLI, a user types on the command line and sees the result right away.

It is easier for the user to get it right on the first try if we only require one field.

I am also open to require create as well, for consistency with the CLI.

Copy link
Member

@doriable doriable left a comment

Choose a reason for hiding this comment

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

This looks good to me, let some comments and a couple of notes:

  • should add to the README
  • I agree that we can just have the create_visibility input to capture both --create and the visibility.

push.bash Outdated Show resolved Hide resolved
Copy link
Member

@cyinma cyinma left a comment

Choose a reason for hiding this comment

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

generally lgtm with few nits, in practice, creating resources require at least organization admin role, and I wonder how many user will put an admin token into the Github action, but this still feel useful to me

push.bash Outdated Show resolved Hide resolved
push.bash Outdated Show resolved Hide resolved
oliversun9 and others added 2 commits June 6, 2023 13:46
Co-authored-by: Tommy Ma <33741651+cyinma@users.noreply.github.com>
@oliversun9 oliversun9 merged commit 342fc4c into main Jun 6, 2023
6 checks passed
@oliversun9 oliversun9 deleted the osun/tcn-1794-buf-push-action-should-support-create branch June 6, 2023 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants