Skip to content

Change behavior of slice flags for issue edit and pr edit commands#2949

Merged
samcoe merged 1 commit intotrunkfrom
edit-improvements
Feb 12, 2021
Merged

Change behavior of slice flags for issue edit and pr edit commands#2949
samcoe merged 1 commit intotrunkfrom
edit-improvements

Conversation

@samcoe
Copy link
Contributor

@samcoe samcoe commented Feb 11, 2021

This PR changes issue edit and pr edit flags behavior. Previously, flags that accepted arrays of values (--label, --reviewer, --project, and --assignee) would overwrite the current values for these fields, this changes those flags to now be piecemeal changes to these fields through the definition of --add and --remove variations of those flags.

For example: The --label flag is now split into two flags --add-label and --remove-label.

@samcoe samcoe self-assigned this Feb 11, 2021
@samcoe samcoe changed the title Change behavior or slice flags for issue edit and pr edit commands Change behavior of slice flags for issue edit and pr edit commands Feb 11, 2021
@samcoe samcoe force-pushed the edit-improvements branch 4 times, most recently from 0e300b7 to 46bf650 Compare February 11, 2021 04:44
This was referenced Feb 11, 2021
@samcoe samcoe marked this pull request as ready for review February 11, 2021 19:16
@samcoe samcoe requested a review from a team February 11, 2021 19:16
"github.com/shurcooL/githubv4"
)

type Editable struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The addition of add and remove was starting to make this struct excessively big which is why I decided to break it down further which resulted in much of the code changes in this PR.

Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

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

This is great!

One final thought: unlike with other metadata fields, it actually makes sense to replace the whole set of assignees, e.g. to reassign from one user to anther. Right now, to achieve that you would need to gh pr edit --add-assignee foo --remove-assignee bar, but for that you need to spell out the name of the previous assignee. Should there be another flag, something like gh pr edit --reassign foo? This is not strictly necessary; I'm just anticipating our users' needs.


if opts.Interactive && !opts.IO.CanPrompt() {
return &cmdutil.FlagError{Err: errors.New("--tile, --body, --assignee, --label, --project, or --milestone required when not running interactively")}
return &cmdutil.FlagError{Err: errors.New("field to edit flag required when not running interactively")}
Copy link
Contributor

Choose a reason for hiding this comment

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

🌟

}

Metadata api.RepoMetadataResult
type EditableSlice struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Love this ✨

Base automatically changed from pr-edit to trunk February 12, 2021 18:21
@samcoe samcoe force-pushed the edit-improvements branch 4 times, most recently from 5c31e38 to 3d0e94a Compare February 12, 2021 21:54
@samcoe
Copy link
Contributor Author

samcoe commented Feb 12, 2021

@mislav I can see a --clear-assignee flag (or some other wording) being useful, lets see what feedback we get. It would be pretty simple to implement if it is highly requested.

@samcoe samcoe merged commit 4e5aa91 into trunk Feb 12, 2021
@samcoe samcoe deleted the edit-improvements branch February 12, 2021 23:02
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