Git commit messages

I just looked at git log and I see:

ommit 85d5a7e8ef472a4a64e5de883cf313c111a8ec77 (HEAD)
Author: Ashwin Ramaswami <aramaswamis@gmail.com>
Date:   Tue Dec 6 08:37:41 2022 -0500

    bpo-37860: re-add netlify.toml to set up deploy previews for docs (#92852)
    
    * Revert "bpo-46184: remove `netlify.toml` (#30272)"
    
    This reverts commit fbaf2e604cd354f1ebc6be029480010c6715a8ca.
    
    * Delete runtime.txt
    
    * Create runtime.txt
    
    * Delete runtime.txt
    
    * Update netlify.toml
    
    * Update netlify.toml
    
    * Add netlify badge
    
    * Update Doc/tools/templates/layout.html
    
    Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
    
    * Update layout.html
    
    Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>

I don’t know what is netlify.toml, I don’t get if this change adds or removes the file. Moreover, it says that it both creates and removes runtime.txt file :frowning: That’s misleading. “Co-authored-by: Hugo” is duplicated. Such commit message is hard to read. I don’t want to blame the person who merged this very specific commit. Such commit message is becoming more and more frequent.

Would you mind to please manually edit the commit message to make it shorter when merging a PR? Remove useless details and try to better explain the change. Thanks :wink:

Another example:

commit 22d91c16bb03c3d87f53b5fee10325b876262a78
Author: Skip Montanaro <skip.montanaro@gmail.com>
Date:   Tue Nov 22 12:06:36 2022 -0600

    gh-99146 struct module documentation should have more predictable examples/warnings (GH-99141)
    
    * nail down a couple examples to have more predictable output
    
    * update a number of things, but this is really just a stash...
    
    * added an applications section to describe typical uses for native and machine-independent formats
    
    * make sure all format strings use a format prefix character
    
    * responding to comments from @gpshead. Not likely finished yet.
    
    * This got more involved than I expected...
    
    * respond to several PR comments
    * a lot of wordsmithing
    * try and be more consistent in use of ``x`` vs ``'x'``
    * expand examples a bit
    * update the "see also" to be more up-to-date
    * original examples relied on import * so present all examples as if
    
    * reformat based on @gpshead comment (missed before)
    
    * responding to comments
    
    * missed this
    
    * one more suggested edit
    
    * wordsmithing

I’m not sure that the keeping the history of the change in the commit message is worth it :wink:

4 Likes

At work, we’ve had good experiences with GitHub’s “Default to pull request title and description” setting; the PR description typically describes more of the “why” of the change than the “how”, and that information is useful to have in the commit message for the squash-merge. It helps if you’re fussy about the PR descriptions in the first place, or willing to take a bit of time to edit it before merge (there’s usually not a lot of editing required).

5 Likes

:100:

Please, please, PLEASE edit the commit message before merging. :pray: :pray: :pray:

5 Likes

The idea of squashing on merge is to get rid of the noise that accumulates from a lot of little steps edging up to an accepted PR. IMO, it partly defeats the purpose if a narrative of that noise is still in the commit msg :slight_smile:

2 Likes

This particular PR wasn’t merged by any of the core devs, so they may not realize our workflow about commit message.

For reference, we do have a checklist when merging a PR, and part of the checklist is to clean up the commit message. I expect other core devs are already familiar with this.

I also think it would be worth adding a style guide on how to write good commit message for CPython.

1 Like

The same issue does come up frequently for core devs too, because of the bad defaults of github’s user interface. The extension Refined Github can help with that. I think automerging does not have the issue, the PR description is used as commit message IIRC, so if that’s clean enough before the label is applied, the commit message is good.

Big +1 from me.

Can we use this setting for the GitHub cpython project?

2 Likes

By the way, the “automerge” bot also produces verbose commit messages which could be enhanced:

commit dfc2732a57e3ea6603d62f769d4f9c80be726fa4
Author: Sam Ezeh <sam.z.ezeh@gmail.com>
Date:   Sun Nov 27 17:58:39 2022 +0000

    gh-91340: Document multiprocessing.set_start_method force parameter (GH-32339)
    
    
    
    #91340
    
    
    https://bugs.python.org/issue47184
    
    Automerge-Triggered-By: GH:kumaraditya303

It does repeat the GitHub issue number in the commit title and in the message. It includes the URL which is supposed to be a comment. It adds multiple useless empty lines.

I reported the empty lines issue more than one year ago in the miss-islington project but sadly nobody is available to attempt fixing it :frowning: