Skip to content

Allow editing commit subject when squash-merging a PR#3344

Merged
mislav merged 5 commits intocli:trunkfrom
cristiand391:allow-edit-commit-subject
Dec 20, 2021
Merged

Allow editing commit subject when squash-merging a PR#3344
mislav merged 5 commits intocli:trunkfrom
cristiand391:allow-edit-commit-subject

Conversation

@cristiand391
Copy link
Contributor

Ref. #1023 (comment)

This PR adds support for editing the commit subject line when squash-merging a PR.

if err != nil {
// Tolerate this API missing in older GitHub Enterprise
if strings.Contains(err.Error(), "Field 'viewerMergeHeadlineText' doesn't exist") {
return "", nil
Copy link
Contributor Author

@cristiand391 cristiand391 Apr 1, 2021

Choose a reason for hiding this comment

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

Same as viewerMergeBodyText, the viewerMergeHeadlineText field might not be available in older GHE versions.

cmd.Flags().BoolVarP(&opts.DeleteBranch, "delete-branch", "d", false, "Delete the local and remote branch after merge")
cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "Body `text` for the merge commit")
cmd.Flags().StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file`")
cmd.Flags().StringVarP(&opts.Subject, "subject", "", "", "Subject `text` for the merge commit")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this be --title?
Since other gh commands already use it I decided to go with --subject but I think title is more descriptive.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think --subject is fine

@mislav mislav self-assigned this Apr 22, 2021
@masoodaam

This comment has been minimized.

@masoodaam

This comment has been minimized.

@mislav mislav removed their assignment Jun 18, 2021
@mislav mislav requested review from samcoe and vilmibm June 18, 2021 20:23
Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

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

thanks for working on this, it's looking good!

I think it would be perfectly fine to allow this functionality on all the merge types -- just like the web UI.

cmd.Flags().BoolVarP(&opts.DeleteBranch, "delete-branch", "d", false, "Delete the local and remote branch after merge")
cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "Body `text` for the merge commit")
cmd.Flags().StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file`")
cmd.Flags().StringVarP(&opts.Subject, "subject", "", "", "Subject `text` for the merge commit")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think --subject is fine

@mislav mislav added the enhancement a request to improve CLI label Aug 4, 2021
@mislav mislav self-assigned this Aug 4, 2021
@mgabeler-lee-6rs
Copy link
Contributor

I & my team greatly miss this feature for our workflow, is there anything I can do to help this along?

@cristiand391
Copy link
Contributor Author

@mgabeler-lee-6rs will see if I can update this PR this weekend :)

@bernhardkaindl
Copy link

bernhardkaindl commented Oct 6, 2021

@cristiand391 Your current commit 6e5db7 from April 1st on the branch of this PR didn't work for me. I

git describe --tags
v0.12.0-1076-g6e5db706
make install prefix=$HOME/.local
$HOME/.local/bin/gh pr merge --squash --auto --subject 'py-ipykernel: Add version 6.0.2 (#26501)' --body ''

resulted in this commit: spack/spack@2cdfa33
The merged commit message subject was the commit message subject of the submitter of the PR, which was not what I expected to see using the command above to do the auto-merge. Even a normal merge with --squash didn't work:

/home/kaindlb/.local/bin/gh pr merge --squash --subject 'py-ipywidgets: Add version 7.6.3 (#26502)'Squashed and merged pull request #26502 (py-ipywidgets: Add version 7.6.3)

Result: spack/spack@943f1e7

@mislav mislav added the external pull request originating outside of the CLI core team label Dec 20, 2021
@cristiand391 cristiand391 requested a review from a team as a code owner December 20, 2021 18:12
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.

Thanks for your patience! I have resolved the merge with trunk, fixed the problem with --subject not being respected in non-interactive mode, and cleaned up the code a little bit.

@mislav mislav enabled auto-merge (squash) December 20, 2021 18:21
@mislav mislav merged commit f2d23d8 into cli:trunk Dec 20, 2021
@cristiand391 cristiand391 deleted the allow-edit-commit-subject branch December 24, 2021 01:30
@Brooklinellcstorecom

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement a request to improve CLI external pull request originating outside of the CLI core team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants