FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-44806: Fix issue with typing.Protocol __init__ method generation by uriyyo · Pull Request #27541 · python/cpython · GitHub

Repository navigation

bpo-44806: Fix issue with typing.Protocol __init__ method generation - #27541

Closed
uriyyo wants to merge 3 commits into
python:mainfrom
uriyyo:fix-issue-44806
Closed

uriyyo wants to merge 3 commits into
python:mainfrom
uriyyo:fix-issue-44806

Conversation

uriyyo commented Aug 2, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

uriyyo commented Aug 2, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

@Fidget-Spinner Could you please review this PR?

I curios about why github did not add you as a reviewer? I can see that you at at CODEOWNERS for typing but every time it's add only Guido.

uriyyo commented Aug 2, 2021

Copy link
Copy Markdown
Member Author

@serhiy-storchaka Could you please review this PR? I have covered case that you mentioned at a issue tracker.

Copy link
Copy Markdown
Member

I curios about why github did not add you as a reviewer? I can see that you at at CODEOWNERS for typing but every time it's add only Guido.

CODEOWNERS only works for people with write access, not for triagers :).

Also sorry, I won't be able to review this PR in time for 3.10rc1. So I'll let Serhiy decide. Both of your PRs look quite similar, just his searches __mro__ and yours searches __bases__.

ambv commented Aug 2, 2021

Copy link
Copy Markdown
Contributor

Closing in favor of GH-27545.

ambv closed this Aug 2, 2021

Copy link
Copy Markdown
Member

I am sorry, @uriyyo. I started working on a patch at a time with you, and when I finished, I saw your PR, but since my PR covered more cases and contained more tests, I kept it for comparison and discussion. Then I had a problem with connection (resolved only few hours ago), so I did not have opportunity to discuss your PR.

In general, they are almost same. There are subtle differences in corner cases, but for now I cannot say what is more correct.

uriyyo commented Aug 3, 2021

Copy link
Copy Markdown
Member Author

@serhiy-storchaka No worries, I am happy that this issue has been resolved🙂

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

6 participants


Back | FazBrowse Home | New Git URL