Skip to content

Conversation

@shields-fn
Copy link
Contributor

Instead of giving an example of using sys and then, at the end, advising to not use sys, just give a preferred example. This is especially helpful since mypy 0.740 has started (correctly) complaining about sys._called_from_test not being present.

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

as we dont have a executing example yet this is fine as is, but we should follow up with an actual execution at a later point

else:
# called "normally"
...
Copy link
Member

Choose a reason for hiding this comment

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

i noted that we haven't yet a actual example execution block, so this example isn't actually executed demoing the effect -

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do I do that?

Copy link
Member

Choose a reason for hiding this comment

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

TBH I think the current example is fine, it is a really contrived use case anyways. 🤷‍♂

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

I think this is mergeable as is but will defer clicking the button until @RonnyPfannschmidt has had additional input 👍

@nicoddemus
Copy link
Member

We should squash the commits before merging as well.

Instead of giving an example of using sys and then, at the end,
advising not to use sys, just give a correct example. This is
especially helpful since mypy 0.740 has started (correctly) complaining
about sys._called_from_pytest not being present.
@RonnyPfannschmidt
Copy link
Member

please merge, having it as runner example take a bit of tinkering

@nicoddemus nicoddemus merged commit e2a0987 into pytest-dev:master Nov 14, 2019
@shields-fn shields-fn deleted the patch-1 branch November 14, 2019 20:17
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.

4 participants