bpo-37004: Documented asymmetry of string arguments in difflib.SequenceMatcher for ratio method

[sweeneyde]

https://bugs.python.org/issue37004

Automerge-Triggered-By: @tim-one

[the-knights-who-say-ni]

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

[tim-one]

I like the idea! But want some changes, as spelled out in comments. Thanks!

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[sweeneyde]

I have made the requested changes; please review again.

[bedevere-bot]

Thanks for making the requested changes!

@tim-one: please review the changes made to this pull request.

[sweeneyde]

What is the process for getting this PR merged?

[tim-one]

Thanks for speaking up! While the change is fine by me, I'm not familiar enough with current workflow to finish it. So I just posted a message to the python-dev mailing list, asking for someone else to proceed:

https://mail.python.org/archives/list/python-dev@python.org/thread/TFTQCJ7IZNCE3XUWEC4IVHPXBDZIKVEI/

I bet they'll ask that you add your name to the Misc/ACKS file.

[miss-islington]

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

[miss-islington]

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

[miss-islington]

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

[miss-islington]

Thanks @sweeneyde for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7, 3.8.
🐍🍒⛏🤖

[bedevere-bot]

GH-15157 is a backport of this pull request to the 3.8 branch.

[bedevere-bot]

GH-15158 is a backport of this pull request to the 3.7 branch.

[bedevere-bot]

GH-15159 is a backport of this pull request to the 3.6 branch.

[terryjreedy]

@tim-one The backports have to be 1. approved by a coredev, after briefly checking that done correctly, in which case the bot will merge if and when CI passes, or 2. directly merged by human, after CI passes. I did the latter for 3.8 and 3.7.

[tim-one]

Thanks, Terry! I was just going to kill them, to avoid screwing things up worse ☹️

While you're here, do you understand what the earlier

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

messages were trying to communicate?

And, when you did the merges, do you understand what the

Please replace # with GH- in the commit message next time. Thanks!

messages were trying to say? I've seen the latter message every time I've "squashed & merged" anything, and still have no idea what it's talking about (I didn't use "#" in the commit messages).

[terryjreedy]

1. Good thing you left them alone. They were fine. If closed, new backport labels here would have been needed. I open this and took a look at them because your fairly detailed explanation of what you did omitted mention of approving the backports.

2. Once CI for a patch to master passes, changes to master can create merge conflicts. When so, Github adds a temporary message to the CI detail box. (It is now shrunken, with a [View Details] button.) One can resolved them by editing files with conflict markers either with Github's online editor or with any editor on one's working machine, followed by a push of the edited branch.

When you added the Automerge label, our backport/merge bot, Miss Islington gets involved. I resume its message paraphrases what I said above. But I am puzzled because there is no listed merge-resolution commit. Perhaps there was a spurious conflict that disappeared, perhaps github retried with a different and more successful merge strategy, perhaps there was just a timing issue. Automerge is fairly recent, and I don't use it for changes to master because it hides the human who made the decision, and I often decide to merge after a final review.

3. The title at the top of this page has the PR number, here bpo-37004: Documented asymmetry of string arguments in difflib.SequenceMatcher for ratio method #13482, at the end, in gray. It cannot be edited. When one presses the green [Merge] button, github adds the number in ()s, such as (bpo-37004: Documented asymmetry of string arguments in difflib.SequenceMatcher for ratio method #13482) in black, and editable. We are supposed to remember to change '#' to 'GH-', because bpo-37004: Documented asymmetry of string arguments in difflib.SequenceMatcher for ratio method #13482 could also refer to a tracker (bpo) number. Automerge does that. (What I don't like about automerge for original PRs is that it hides who triggered the merge.)

The same change is requested for backport numbers on backport merges (although there is discussion that they should maybe be deleted instead, or maybe the original PR #). I forgot.

A coredev approving a backport is equivalent to an automerge label. Then the change is automatic. I could have used that instead. I have not checked yet whether the label is available.

[tim-one]

Thanks again, Terry. Ya, I assumed the "can't merge this PR" messages were complaining about a merge conflict, but didn't see any evidence of such. Plus there were three such messages, two of which followed the bot saying it merged the commit into master.

So then I thought "OK, 3 complaints, but nothing fishy about master, but I added 3 backport labels so it must be complaining about them". But no sign of merge conflicts in those either.

For the rest, sounds like I'll avoid automerge in the future. It's hard enough trying to guess the consequences of what I explicitly do 😉.