Fixing commit message after merge

(Cheryl Sabella) #1

I just merged a pull request and even though I removed all the commit messages before the Squash and Merge, I somehow got an error when I clicked the button and it didn’t use my edited version. Is there anyway to remove these now that it’s merged? Thanks!

Here’s the PR:

0 Likes

(Pablo Galindo Salgado) #2

Sadly no, because that would imply rewriting history and you need to force push to master to do that (and only GitHub admins can do that). :frowning:

This happened to me as well:

What I do now is to refresh the page and it GitHub complains about something when clicking merge I just open a new tab/window to avoid this problem.

Probably not a satisfactory answer, I know :slight_smile:

3 Likes

(Cheryl Sabella) #3

Thank you for the explanation. I’ve been so careful to double check every step before hitting the button, so I was really surprised when I got the message. I hope it doesn’t happen again, but I’ll keep your advice in mind if it does. Thanks! :slight_smile:

3 Likes

(Ned Deily) #4

I think we all eventually end up with at least one unexpected commit message that we’d like to edit. Don’t worry too much about it. What’s more important is what ends up in the NEWS entry (usually via blurb) since that ends up in the changelog included in the doc set. The good news there is that that can be edited at any time since it exists in the repo in a file. Which file will vary: as part of each release, the individual NEWS files from each PR merged are consolidated into one file per release. If you discover a problem in a NEWS entry and need help fixing it, contact a friendly neighborhood release manager for assistance. “We do blurb.”

1 Like

(Inada Naoki) #5

One way to avoid it is using “automerge” label.

PR title and body are used for commit message.
So you can edit PR title and body. After you’re satisfied with them, you can add “automerge” label.

example commit:

3 Likes

(Cheryl Sabella) #6

Thank you, Ned and Inada-san for the additional suggestions. I especially like the idea of the automerge since I’m not really sure what happened this time, so, beyond refreshing as Pablo suggested, I’m a bit worried it will happen again. Looking on the bright side, I didn’t break the entire build. :slight_smile:

0 Likes

(Victor Stinner) #7

I dislike automerge because it’s unclear what will be the commit message. I prefer to review the commit message and commit title manually before a merge. It’s common that a PR has a long history with many commits, and we usually only keep a short summary of the change, not the whole history, once the PR is merged.

0 Likes

(Brett Cannon) #8

Whatever the first comment on the PR is will become the commit message.

0 Likes

(Victor Stinner) #9

Well, let me show you an example:

commit 6955d44b41058e3bcc59ff41860bd4cc8948c441
Author: Lorenz Mende <Lorenz.mende@gmail.com>
Date:   Wed Apr 10 19:19:16 2019 +0200

    bpo-34144: Fix of venv acvtivate.bat for win 10 (GH-8321)
    
    
    
    The script needs to be updated to support win 10/ 1803 chcp.com command (output has trailing dot)
    
    
    
    
    
    https://bugs.python.org/issue34144

It’s easy for me to spot commits merged by “automerge”. I don’t see the point of keeping “https://bugs.python.org/issue34144” link: it’s redundant with “34144”. I don’t understand why there are so many empty lines. I would be nice to try to reformat to 80 columns.

The commit comes from: https://github.com/python/cpython/pull/8321/

0 Likes

(Brett Cannon) #10

There isn’t a point, but no one has tried to add any smarts to either automerge or Bedevere to leave the link out.

I suspect it’s from automerge not collapsing newlines when it strips out <!-- --> comments from the issue template. I suspect if someone added code to do a re.sub(r"\n\n+", "\n\n", message) it would get you the result you’re after.

Patches welcome! :wink: But honestly it doesn’t bother me personally.

0 Likes

(Victor Stinner) #11

Yeah, that’s also what I understood but this issue doesn’t bother me enough to motivate me to look into the code :slight_smile:

0 Likes