add --discussion-category flag to release cmd#4003
Conversation
Flag for signaling that a discussion should be created with the given category for the release. Discussions are not supported for draft releases. If a discussion category is given for a draft, an err will be shown. Closes cli#3381
|
Info on API for releases: |
mislav
left a comment
There was a problem hiding this comment.
Thanks for the code and for your patience! I have a couple of minor requests.
Can't wait for this to ship so we can dogfood is from our own release automation!
cli/.github/workflows/releases.yml
Line 162 in c5371d5
| if cmd.Flags().Changed("discussion-category") && opts.Draft { | ||
| return errors.New("Discussions for draft releases not supported") |
There was a problem hiding this comment.
Will the server also error out on this? If yes, I would propose removing the client-side validation. Maybe in the future the platform allows this (e.g. saves the parameter in a draft release and uses it upon publish), and in that case this bit of client-side validation will be unwanted.
There was a problem hiding this comment.
Just tested this. The server creates the release and ignores the discussion category. No message or anything from the server that the discussion category is not allowed for draft releases. Maybe worth relaying that to the backend team or w/e as that might be confusing, especially if you're trying to automate this stuff. I'll leave in this validation and push changes for the rest of your comments. Look forward to hearing your feedback.
There was a problem hiding this comment.
It's fine to leave the validation for now. I don't think it's extremely likely that the platform will accept and respect this parameter for draft releases anytime soon.
pkg/cmd/release/create/create.go
Outdated
| // maximum number of simultaneous uploads | ||
| Concurrency int | ||
|
|
||
| DiscussionCategoryName string |
There was a problem hiding this comment.
Nit: let's call this DiscussionCategory so it's not overly long!
pkg/cmd/release/create/create.go
Outdated
| } | ||
| } | ||
|
|
||
| if opts.Draft && len(opts.DiscussionCategoryName) > 0 { |
There was a problem hiding this comment.
This validation is redundant if the RunE block already performs validation of cli inputs
There was a problem hiding this comment.
I think you'd still need to perform validation after the interactive bit on line 252, because the value of opts.Draft could change.
pkg/cmd/release/create/create.go
Outdated
| "prerelease": opts.Prerelease, | ||
| "name": opts.Name, | ||
| "body": opts.Body, | ||
| "discussion_category_name": opts.DiscussionCategoryName, |
There was a problem hiding this comment.
Should we only add this param if the flag was set? That way we avoid having to change a bunch of old tests
Only add discussion category to request if there is one. This eliminates the need to update old tests. Renaming the variable to something shorter.
mislav
left a comment
There was a problem hiding this comment.
It looks great. Thank you so much!
Flag for signaling that a discussion should be created with the given
category for the release. Discussions are not supported for draft
releases. If a discussion category is given for a draft, an err will be
shown.
Closes #3381