RFE automerge enhancements

The “automerge” label could generate better commit messages.

Example of commit messages generated by automerge:

commit c5b242f87f31286ad38991bc3868cf4cfbf2b681
Author: Ashwin Ramaswami <[aramaswamis@gmail.com](mailto:aramaswamis@gmail.com)>
Commit: Miss Islington (bot) <[31488909+miss-islington@users.noreply.github.com](mailto:31488909%2Bmiss-islington@users.noreply.github.com)>
Date:   Sat Aug 31 10:25:35 2019 -0500

bpo-37764: Fix infinite loop when parsing unstructured email
headers. (GH-15239)

Fixes a case in which email._header_value_parser.get_unstructured
hangs the system for some invalid headers. This covers the cases in
which the header contains either:
- a case without trailing whitespace
- an invalid encoded word

https://bugs.python.org/issue37764

This fix should also be backported to 3.7 and 3.8

https://bugs.python.org/issue37764
  • very long lines: usually, commit messages are wrapped to use 80 columns maximum
  • 3 empty lines then 2 empty lines, why? 1 empty line would be enough :slight_smile:
  • https://bugs.python.org/issue37764” link is redundant with “bpo-37764:”
  • https://bugs.python.org/issue37764” link is written twice (this issue might be specific to this specific PR/change)

The bot doesn’t log who approved the PR: “Commit: Miss Islington
(bot) <31488909+miss-islington@users.noreply.github.com>” is not
helpful. I would prefer to have directly the information in Git, rather than depending on GitHub (we might migrate to yet another forge in 10 years).

Victor

4 Likes

If I remember correctly the last time I looked this up I couldn’t find a way to specify the committee in git itself. Best you can get is a line in the commit message.

Oh, I’m perfectly fine with having it in the commit message. The minimum would be to include the GitHub handle. The best would be to have a format like "Full Name ".

Just to be clear, is it fair to say that your concern here is with the commit messages created by the new feature of automerging initial commits (to master) rather than the more familiar feature of automerging backports, where Miss Islington uses the commit message from the original, manually-committed and -edited merge as a starting point?

I saw issues of commit messages with pull requests proposed on the master branch and merged using the “automerge” label added manually.

I understood that the pull request title and description are used to build the commit message: but the function which does that could be enhancement. Moreover, I’m not sure that users of the automerge label understood that properly.

I see now that the changes have already been made to add the “Automerge Triggerred by: @handle” in the commit messages, like this one.

It would be great if we didn’t use the “@” since Github tends to consider it as a mention, even in commit messages, even in forks and tends to send out notification emails.

For example, this one of the emails I received about a commit merged:

bpo-32793: Fix a duplicate debug message in smtplib (GH-15341)

_get_socket() already prints a debug message for the host and port.

https://bugs.python.org/issue32793

Automerge-Triggered-By: @maxking

-- 
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/PatrikKopkan/cpython/commit/c35c387bccf9b8447c9051f5d332c4770ccd08cb

I hope GitHub won’t send out notifications if we don’t use the “@”.

1 Like

I think this is a feature, not a bug. The bpo-37764 is good in the title, where space is limited, but spelling out the full link is very useful for:

  • new people who tend to be overwhelmed by abbreviations.
  • automatic linkifying (even if I install an extension, my terminal or GitHub don’t know about bpo)
  • future archaeologists, if bpo is abandoned and the abbreviation forgotten
1 Like

It won’t, so it’s a question on whether the automatic linking in the commit log when viewed on GitHub and the notifications are a good or bad thing.

I wouldn’t call it a bad thing, but I don’t think the minimal benefit outweighs the spam from “mention” emails.

Oh, nice! It would be even better with the full name :wink:

1 Like

It would solve my notification problem, but if the original intent is to lookup who merged it, handles are unique and easier to search than Full Names in Github.

I was thinking that we can write both. For example:

Automerge-Triggered-By: Abhilash Raj (@maxking)

I’m unable to recall the GitHub handle of all core devs, and I’m sure that people who are external to Python have even more troubles to recognize GitHub handles.

Some core devs use different nickanmes on the bug tracker, GitHub and IRC.

To help with fixing this concern, I was working on https://github.com/python/devguide/issues/507. It would be useful to have the full listing on the devguide, but I figured that starting with the experts (who are the most likely to be needed in PRs) would be significantly easier. Currently awaiting review on the first PR in that process, which adds a JSON file to the devguide https://github.com/python/devguide/pull/517 (as suggested by Mariatta and Zach Ware).

In the meantime, I usually refer to the committers list on bpo. But there’s not an easy way to link that to the experts index on the devguide, which was the main purpose of the JSON. Also, since we’re planning on moving away from bpo, that should probably not be the only location where GitHub handles are linked to the names of core developers.

But, for the purpose of the automerge enhancement, it’s probably better to just use the full names. The issue with looking up GitHub handles is not limited to this alone though, particularly for newer contributors or for those external to the organization.