Skip to content

Add --branch to buf push#2457

Merged
doriable merged 8 commits intomainfrom
BSR-2470-add-branch-flag
Oct 4, 2023
Merged

Add --branch to buf push#2457
doriable merged 8 commits intomainfrom
BSR-2470-add-branch-flag

Conversation

@doriable
Copy link
Member

This aliases the --draft flag to --branch in buf push.
We are planning to start branch support with parity to --draft.
The description is also updated for the flag to reflect branch.

@doriable doriable requested a review from bufdev September 18, 2023 18:08
flagSet.SetNormalizeFunc(aliasDraftToBranchFunc)
}

// aliasDraftToBranchFunc aliases the draft flag to branch.
Copy link
Member

Choose a reason for hiding this comment

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

  • are we sure we want to directly alias, and have no change in behavior?
  • is there not a native way in pflag/cobra to do this?

Copy link
Member Author

Choose a reason for hiding this comment

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

For the initial step of releasing the branches UI, yes. There are server side changes we need to make for upcoming features, but it doesn't warrant a flag structure change.

This is the recommended way from Cobra's pflag package: https://github.com/spf13/pflag#mutating-or-normalizing-flag-names

Copy link
Member

Choose a reason for hiding this comment

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

Once we do this, it's forever, so we can't later bring in any changes to --branch - just confirming we're comfortable with this

Copy link
Member Author

Choose a reason for hiding this comment

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

I had an original draft of this change where I added a new --branch flag, and then had a check return an error if the user attempted to pass both a --draft and --branch.
The normalize func here seems to be the recommended way to do a direct alias, but I think I would be more comfortable with the new flag and the check. Does that seem reasonable to you?

Copy link
Member

Choose a reason for hiding this comment

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

Yea I would error if both are specified.

CHANGELOG.md Outdated
- Update `buf beta price` with the latest pricing information.
- Fix issue where `buf generate --exclude-path` was not properly excluding paths
for remote modules.
- Alias the `--draft` flag for `buf push` to `--branch` and update flag description.
Copy link
Member

Choose a reason for hiding this comment

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

I would not call out the alias. More specifically, I would actually only do this when you are ready to deprecate --draft entirely, and then deprecate and hide --draft, and add `--branch.

@doriable doriable requested a review from bufdev October 3, 2023 17:20
@bufdev bufdev changed the title Alias --draft flag to --branch in buf push Add --branch to buf push Oct 3, 2023
@doriable doriable merged commit ff74f32 into main Oct 4, 2023
@doriable doriable deleted the BSR-2470-add-branch-flag branch October 4, 2023 15:17
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.

2 participants