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
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).
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 “@”.
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.
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.
I’m concerned that a contributor might hijack the full name of someone else on purpose, or more simply than two people can have the same full name. Recently, the rockstar Ben Harper contributed to Python!!! https://bugs.python.org/issue36356
For the author of a commit, we have the email address which is unique. So I suggest to store the full name + GitHub handle.
Ah, good point. Name collision is probably the more likely of the two. I honestly hadn’t considered that there are enough contributors for full name collisions to be an issue, but I suppose that’s a good thing. (:
Hey, you never know.
Are your thoughts similar on storing the expert data? I was thinking that the expert index in the devguide should use a platform-independent identifier, in case we move away from GitHub at some point in the distant future or need the expert info for other platforms. Since Python has already outlived several other VC platforms, it wouldn’t surprise me if we outlived GitHub as well.
An email address would be a better unique identifier, my only concern is that some of the experts may not want to publicly list their email address (this could be fixed by assigning all of them an @python.org email). The full name collision is a bit less of an issue for the experts index than it would be automerge, but it never hurts to be more scalable.
IMO we don’t necessarily need to do this since experts index is platform dependent when it comes to requesting review/tagging someone. Today, we use it for nosying experts on BPO and hence it uses bpo usernames, I can’t do that with email addresses.
When we move the issues to Github, perhaps we can just use the Github username. If the index is converted to a json/toml format, it can easily be changed by automation in future, when needed, from either the voter repo + some manul work for non-core-devs in the experts index.
That’s exactly what I was aiming to do with Add JSON for storing expert data by aeros · Pull Request #517 · python/devguide · GitHub. On the related issue page, @zware linked the script that was used by bpo to grab experts for the nosy list. If that PR is merged, I would be able to adjust the script to pull from the json instead of the experts index page. That would allow us to add GitHub usernames (in addition to the bpo usernames) to the table on experts.rst without having to worry about conflicts.
Can that be the GH handle without the @? For the example above:
Automerge-Triggered-By: Abhilash Raj (maxking at GitHub)
That way GH won’t send notifications. It also won’t linkify it, but I think that’s not a big loss.
Also, this way will be much more useful if we ever move away from GitHub.
