Skip to content

Conversation

@Akuli
Copy link
Collaborator

@Akuli Akuli commented May 15, 2021

Combination of #5460 and #5458

@Akuli
Copy link
Collaborator Author

Akuli commented May 15, 2021

@srittau This is the combined PR. When going through the conflicts, I noticed that you generally did this better than my script, although the script handled one version check that you had missed.

@srittau
Copy link
Collaborator

srittau commented May 15, 2021

Give me a sec, I'm still going through my PR a second time to see if there are any errors.

@Akuli
Copy link
Collaborator Author

Akuli commented May 15, 2021

Clicking the merge commit above shows how this PR differs from yours. The version check you missed is in stdlib/warnings.pyi. The unix_dialect you already fixed is in stdlib/csv.pyi.

Given that this is a combination of something done with a script and something done by a human, this quite likely catches missing things (script) and handles comments and funny corner cases properly (human).

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the merge! LGTM.

@Akuli Akuli merged commit 17dcea4 into python:master May 15, 2021
@Akuli Akuli deleted the combined branch May 15, 2021 12:33
@srittau
Copy link
Collaborator

srittau commented May 15, 2021

@Akuli Just to avoid double work again (sorry about that!): I would start working on those now-obsolete aliases like _Text etc.

@Akuli
Copy link
Collaborator Author

Akuli commented May 15, 2021

Sure, you can do that. I think this double work wasn't a bad thing, since my PR and your PR both had some problems that the combined PR didn't have.

In the past, I have done a similar thing at work, although slightly more complicated: I and a coworker both did some changes, then we both finished merging them at about the same time (many conflicts), then we together merged our already once merged versions together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants