KEMBAR78
Add support for specifying multiple packages on the command line alongside modules by eric-wieser · Pull Request #4733 · python/mypy · GitHub
Skip to content

Conversation

@eric-wieser
Copy link
Contributor

Fixes #4732

Let me know how best to add tests for this, if you think they're needed.

@gvanrossum
Copy link
Member

gvanrossum commented Mar 14, 2018 via email

@eric-wieser
Copy link
Contributor Author

CI output wasn't showing up when I looked before. I'll take a look later today

@eric-wieser eric-wieser force-pushed the multiple-packages branch 2 times, most recently from 78d430c to 8fdc06a Compare March 15, 2018 01:43
@eric-wieser
Copy link
Contributor Author

eric-wieser commented Mar 15, 2018

Alright, fixed.

@gvanrossum
Copy link
Member

Hm, we don't seem to have tests for -m and -p at all... Could you add some? They would go in test-data/unit/cmdline.test.

@eric-wieser
Copy link
Contributor Author

Tests don't run at all locally for my machine, so I'm gonna hijack the CI to do my testing...

(fails with AssertionError: De-serialization failure: TypeInfo not fixed when I run python setup.py test)

@eric-wieser
Copy link
Contributor Author

Ok, test added

@gvanrossum
Copy link
Member

gvanrossum commented Mar 15, 2018 via email

@eric-wieser
Copy link
Contributor Author

eric-wieser commented Mar 19, 2018

Are you happy with the test I've added here? Seems to pass locally and on CI

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Yes, that's exactly the kind of test I was thinking of. The rest of the code looks good too. I'll ask Ivan if he thinks this makes the cur for this week's release (if not, there'll be another release in 3 weeks).

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.

3 participants