Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reorder entries in Misc/ACKS #17663

Merged
merged 3 commits into from Dec 20, 2019
Merged

Reorder entries in Misc/ACKS #17663

merged 3 commits into from Dec 20, 2019

Conversation

@merwok
Copy link
Contributor

merwok commented Dec 19, 2019

No description provided.

Misc/ACKS Outdated Show resolved Hide resolved
Dan Lidral-Porter
Ngalim Siregar
Tim Gates
Karthikeyan Singaravelan

This comment has been minimized.

Copy link
@merwok

merwok Dec 19, 2019

Author Contributor

12 lines added, 13 lines removed because Ngalim Siregar was already in the right spot.

Copy link
Member

serhiy-storchaka left a comment

Thank you! I was going to do this myself, it was in my low priority list.

@merwok merwok merged commit dd1a20f into master Dec 20, 2019
5 checks passed
5 checks passed
Docs
Details
Azure Pipelines PR #20191219.12 succeeded
Details
bedevere/issue-number Issue report skipped
bedevere/news "skip news" label found
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@merwok merwok deleted the fix-misc-acks-sort branch Dec 20, 2019
@bedevere-bot

This comment has been minimized.

Copy link

bedevere-bot commented Dec 20, 2019

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

@miss-islington

This comment has been minimized.

Copy link

miss-islington commented Dec 20, 2019

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

@miss-islington

This comment has been minimized.

Copy link

miss-islington commented Dec 20, 2019

Sorry, @merwok, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker dd1a20f324d88f4171aca480b7972d68cab212c5 3.8

@miss-islington

This comment has been minimized.

Copy link

miss-islington commented Dec 20, 2019

Sorry @merwok, I had trouble checking out the 3.7 backport branch.
Please backport using cherry_picker on command line.
cherry_picker dd1a20f324d88f4171aca480b7972d68cab212c5 3.7

@bedevere-bot

This comment has been minimized.

Copy link

bedevere-bot commented Dec 20, 2019

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

@bedevere-bot

This comment has been minimized.

Copy link

bedevere-bot commented Dec 20, 2019

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

@merwok

This comment has been minimized.

Copy link
Contributor Author

merwok commented Dec 20, 2019

Thanks for review. If the problem comes again, we could add a comment at the end of the file.

@serhiy-storchaka

This comment has been minimized.

Copy link
Member

serhiy-storchaka commented Dec 20, 2019

Nice idea about a comment at the end of the file!

sthagen added a commit to sthagen/cpython that referenced this pull request Dec 20, 2019
reorder entries in Misc/ACKS (python#17663)
@tirkarthi

This comment has been minimized.

Copy link
Contributor

tirkarthi commented Dec 21, 2019

Sorry, being the latest entry added by myself I read the sorted nature of the file at the top but didn't that it was sorted by last name expecting sorted by first name. It's a good idea to add the comment in the end about the same. I tried adding a check to patchcheck.py since it already has a check for ACKs update to ensure the file stays sorted but hit some cases with unicode names having different value.

Thanks.

@merwok

This comment has been minimized.

Copy link
Contributor Author

merwok commented Dec 21, 2019

The order is not 100% exact, so I think trying to make an automated order would be hard (e.g. if there are three bits in the name, could be two first names or a two-part last name).

Let me make a PR to add the comment at the end and add it to the backports PRs too (and please review these!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.