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

Delete leftover clinic-generated file for C zipimport. #15174

Merged
merged 1 commit into from
Aug 10, 2019

Conversation

gnprice
Copy link
Contributor

@gnprice gnprice commented Aug 8, 2019

This module was rewritten into pure Python in commit 79d1c2e, and
both Modules/zipimport.c and Modules/clinic/zipimport.c.h were
deleted in that commit.

Then a little later, 4a934d4 edited the Argument Clinic script and
made a sweep through its generated files; and it added back a version
of Modules/clinic/zipimport.c.h , presumably by accident.

Nothing includes this file, so it hasn't been having any effect,
but it is a bit confusing. Clean it up.

This module was rewritten into pure Python in commit 79d1c2e, and
both Modules/zipimport.c and Modules/clinic/zipimport.c.h were
deleted in that commit.

Then a little later, 4a934d4 edited the Argument Clinic script and
made a sweep through its generated files; and it added back a version
of Modules/clinic/zipimport.c.h , presumably by accident.

Nothing includes this file, so it hasn't been having any effect,
but it is a bit confusing.  Clean it up.
@gnprice
Copy link
Contributor Author

gnprice commented Aug 8, 2019

/cc @serhiy-storchaka :-)

@gnprice
Copy link
Contributor Author

gnprice commented Aug 9, 2019

Thanks @serhiy-storchaka !

I'm betraying my ignorance of the workflow here, but: what's the next step to merge this? Do you hit the merge button, or does something auto-merge it under certain conditions?

@serhiy-storchaka serhiy-storchaka merged commit 51aac15 into python:master Aug 10, 2019
@miss-islington
Copy link
Contributor

Thanks @gnprice for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 10, 2019
(cherry picked from commit 51aac15)

Co-authored-by: Greg Price <gnprice@gmail.com>
@bedevere-bot
Copy link

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

@serhiy-storchaka
Copy link
Member

Thank you for your contribution @gnprice!

Yes, I just waited until the CI tests be passed before hitting the merge button. I was busy yesterday, so do it today.

miss-islington added a commit that referenced this pull request Aug 10, 2019
(cherry picked from commit 51aac15)

Co-authored-by: Greg Price <gnprice@gmail.com>
@gnprice
Copy link
Contributor Author

gnprice commented Aug 10, 2019

Yes, I just waited until the CI tests be passed before hitting the merge button. I was busy yesterday, so do it today.

Got it. Perfectly sensible, and thanks for the help understanding the workflow!

@gnprice
Copy link
Contributor Author

gnprice commented Aug 10, 2019

(It seems like the pattern is: backport cherry-picks get auto-merged once they're approved and the CI passes. But original PRs to master don't -- a human must merge them. Is that right?

I think I'd seen the former happen on some PRs and so knew that a bot sometimes auto-merged things, but hadn't quite twigged that "backport vs. original non-backport" was the key variable in the pattern.

Anyway, thanks again for your help, and for merging!)

@gnprice gnprice deleted the pr-zipimport-clinic branch August 10, 2019 19:12
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants