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

[3.8] bpo-38122: minor fixes to AsyncMock spec handling (GH-16099). #16137

Closed
wants to merge 2 commits into from

Conversation

matrixise
Copy link
Member

@matrixise matrixise commented Sep 14, 2019

(cherry picked from commit 14fd925)

Co-authored-by: Michael Foord voidspace@users.noreply.github.com

https://bugs.python.org/issue38122

…).

(cherry picked from commit 14fd925)

Co-authored-by: Michael Foord <voidspace@users.noreply.github.com>
@matrixise
Copy link
Member Author

Hi @voidspace

There is problem with the #14700 because that one has not been backported to 3. and your code use _AnyComparer. I try to find the right solution with @pganssle

@matrixise matrixise self-assigned this Sep 14, 2019
@matrixise
Copy link
Member Author

@voidspace _AnyComparer is specific to master, not backported to 3.8, so, I have removed that class from your PR and run the tests. wait & see.

@matrixise matrixise requested a review from lisroach September 26, 2019 09:00
Copy link
Contributor

@lisroach lisroach left a comment

Choose a reason for hiding this comment

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

LGTM

@tirkarthi
Copy link
Member

Seems this missed the RC build :( It more looks like a code refactor. If I understand it correctly in previous case there was list comprehension over arguments starting with "spec" to take first argument and perhaps depending on the order they are passed. Now with PR it checks for spec_set and falls back to spec making the choice predictable to detect. I guess it could cause problem if someone depends on the order in 3.8 RC or 3.8.0 and upgrades to 3.8.1 as it's merged later. Also there is a removed comment about what to do when spec_set and spec are different so could it be tested to ensure this choice is explicit?

@matrixise
Copy link
Member Author

I worked on this issue in september but 3.8 has been released in october, do you think we could merge it into 3.8?

@csabella
Copy link
Contributor

@matrixise, should this change still be backported to 3.8? Thanks!

@matrixise
Copy link
Member Author

@csabella for this PR, yep, I think we should backport it to 3.8.

@ambv
Copy link
Contributor

ambv commented Apr 26, 2021

Sorry, closing this stale backport as 3.8 is closing to security fixes only mode.

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.

8 participants