Member
…pec`
Open
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a user of unittest.mock, but these changes look reasonable to me.
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks reasonable to me as well.
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Supportive of the change, just the code hygiene changes to make.
When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.
Member Author
ncoghlan
left a comment
•
edited
Loading
edited
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I especially like the test coverage.
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Member Author
Thanks everyone! 🎉
Closed
Merged