Skip to content

/cpython

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

bpo-38878: Fix os.PathLike __subclasshook__ #17336

Merged
merged 10 commits into from Dec 22, 2019
Merged

bpo-38878: Fix os.PathLike __subclasshook__ #17336

merged 10 commits into from Dec 22, 2019
+15 −1

Conversation

@bharel
Copy link
Contributor

bharel commented Nov 22, 2019 •
edited by bedevere-bot

Quick subclasshook fix. No need for tests, regression bugs cannot happen and the same method is being used in collections.abc (up to a certain degree).

https://bugs.python.org/issue38878
bharel added 4 commits May 7, 2019
@bharel
Merge pull request #1 from python/master
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
9d2e344 
Merge
@bharel
Merge remote-tracking branch 'upstream/master'
e249b30
@bharel
update master
67f8d4c
@bharel
added a news entry
Loading status checks…
92cc094
@brettcannon brettcannon self-requested a review Nov 22, 2019
@brettcannon
brettcannon requested changes Nov 22, 2019
View changes
Copy link
Member

brettcannon left a comment

Please provide tests so that this functionality isn't accidentally removed in the future.
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Nov 22, 2019

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.
@bharel
Added tests.
Loading status checks…
5466a65
@bharel

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

bharel commented Nov 22, 2019

I have made the requested changes; please review again
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Nov 22, 2019

Thanks for making the requested changes!

@brettcannon: please review the changes made to this pull request.
@bedevere-bot bedevere-bot requested a review from brettcannon Nov 22, 2019
@ilevkivskyi
ilevkivskyi reviewed Nov 22, 2019
View changes
Lib/os.py Show resolved Hide resolved
bharel added 3 commits Nov 25, 2019
@bharel
Changed method lookup to _check_methods
Loading status checks…
67c92c2
@bharel
tiny fix
Loading status checks…
18e8d07
@bharel
another regression assert
Loading status checks…
c06a6cd
@bharel

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

bharel commented Nov 25, 2019

Alright, ready for merge.

I have made the requested changes; please review again
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Nov 25, 2019

Thanks for making the requested changes!

@brettcannon: please review the changes made to this pull request.
@bharel

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

bharel commented Dec 4, 2019

Ready for merge?
@ilevkivskyi

This comment has been minimized.

Sign in to view
Copy link
Contributor

ilevkivskyi commented Dec 4, 2019

Ready for merge?

I will get back to this on the weekend.
@brettcannon brettcannon removed their request for review Dec 4, 2019
bharel added 2 commits Dec 21, 2019
@bharel
Merge remote-tracking branch 'upstream/master'
66e9813
@bharel
Merge branch 'master' into bpo-38878
Loading status checks…
3f76be9
@bharel bharel requested a review from brettcannon Dec 21, 2019
@ilevkivskyi
ilevkivskyi approved these changes Dec 22, 2019
View changes
Copy link
Contributor

ilevkivskyi left a comment

OK, LGTM now.
@ilevkivskyi

This comment has been minimized.

Sign in to view
Copy link
Contributor

ilevkivskyi commented Dec 22, 2019

Also since this is technically a bug, I am going to backport this to 3.7 and 3.8.
@ilevkivskyi ilevkivskyi merged commit eae87e3 into python:master Dec 22, 2019
9 checks passed
9 checks passed
Docs
Details
Windows (x86)
Details
Windows (x64)
Details
macOS
Details
Ubuntu
Details
Azure Pipelines PR #20191221.5 succeeded
Details
bedevere/issue-number Issue number 38878 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@miss-islington

This comment has been minimized.

Sign in to view
Copy link

miss-islington commented Dec 22, 2019

Thanks @bharel for the PR, and @ilevkivskyi for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖
@miss-islington

This comment has been minimized.

Sign in to view
Copy link

miss-islington commented Dec 22, 2019

Sorry, @bharel and @ilevkivskyi, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker eae87e3e4e0cb9a0ce10c2e101acb6995d79e09c 3.8
@miss-islington

This comment has been minimized.

Sign in to view
Copy link

miss-islington commented Dec 22, 2019

Sorry @bharel and @ilevkivskyi, I had trouble checking out the 3.7 backport branch.
Please backport using cherry_picker on command line.
cherry_picker eae87e3e4e0cb9a0ce10c2e101acb6995d79e09c 3.7
@ilevkivskyi

This comment has been minimized.

Sign in to view
Copy link
Contributor

ilevkivskyi commented Dec 22, 2019

@bharel Oops, there are conflicts, you can however do this manually.
sthagen added a commit to sthagen/cpython that referenced this pull request Dec 22, 2019
@sthagen
Merge pull request #113 from python/master
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
1ddf97c 
bpo-38878: Fix os.PathLike __subclasshook__ (pythonGH-17336)
@bharel bharel deleted the bharel:bpo-38878 branch Dec 23, 2019
bharel added a commit to bharel/cpython that referenced this pull request Dec 23, 2019
@bharel
[3.8] bpo-38878: Fix os.PathLike __subclasshook__ (pythonGH-17336)
7c28111 
Quick subclasshook fix using the same method is being used in collections.abc (up to a certain degree)..
(cherry picked from commit eae87e3)

Co-authored-by: Bar Harel <bzvi7919@gmail.com>
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Dec 23, 2019

GH-17684 is a backport of this pull request to the 3.8 branch.
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Dec 23, 2019

GH-17685 is a backport of this pull request to the 3.7 branch.
ilevkivskyi added a commit that referenced this pull request Dec 23, 2019
@bharel @ilevkivskyi
[3.8] bpo-38878: Fix os.PathLike __subclasshook__ (GH-17336) (GH-17684)
Loading status checks…
0846e5d 
https://bugs.python.org/issue38878
ilevkivskyi added a commit that referenced this pull request Dec 23, 2019
@bharel @ilevkivskyi
[3.7] bpo-38878: Fix os.PathLike __subclasshook__ (GH-17336) (GH-17685)
Loading status checks…
59d06b9 
https://bugs.python.org/issue38878
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ilevkivskyi ilevkivskyi

@brettcannon brettcannon

Assignees

@ilevkivskyi ilevkivskyi

Labels
CLA signed
Projects
None yet
Milestone
No milestone
6 participants
@bharel @bedevere-bot @ilevkivskyi @miss-islington @brettcannon @the-knights-who-say-ni
You can’t perform that action at this time.