Skip to content

Simplify Travis-CI configuration #3220

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

Merged
merged 1 commit into from
Sep 30, 2019

Conversation

srittau
Copy link
Collaborator

@srittau srittau commented Sep 3, 2019

This removes the indirection of setting environment variables to
install and run tests. Also, use an explicit version for running
mypy with typed-ast.

This removes the indirection of setting environment variables to
install and run tests. Also, use an explicit version for running
mypy with typed-ast.
@srittau srittau marked this pull request as ready for review September 3, 2019 13:24
Copy link
Contributor

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

TEST_CMD does seem like an unhelpful indirection.

However, the INSTALL does avoid some repetition, so seems helpful to keep?

Also, use an explicit version for running mypy with typed-ast.

This sounds good for future proofing.

@@ -2,34 +2,24 @@ dist: bionic
language: python
python: 3.7

matrix:
jobs:
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this have any significance or are they the same?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As far as I understand it, they are different indeed: In a matrix build, every possible combination of configurations is run, jobs are more straight-forward. Since in our case each jobs is quite different from the others, a matrix build does not make much sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, jobs does sound more appropriate.

env:
- TEST_CMD="./tests/mypy_test.py"
- INSTALL="mypy"
install: pip install -U git+git://github.com/python/mypy
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure typed_ast should be removed here? As I understand it, mypy still depends on it even on 3.8.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That doesn't seem to be the case, since the build succeeds. At least at runtime it should not need it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't check but it probably installs typed_ast from PyPI because it is a dependency of mypy. But the intention was to check with the git typed_ast.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just looked at the travis log. It is indeed installing typed-ast from pypi, but I assume that it will not get used by mypy.

Copy link
Contributor

Choose a reason for hiding this comment

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

I asked about it here -- according to the replies, it it still needed in Python 3.8 to parse Python 2 code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, good point. But this shouldn't be a problem for us, since we are only parsing stubs and are not running mypy in Python 2 mode. I think using typed-ast from pypi is fine, but in the end it doesn't matter much, especially considering that typed-ast will probably not be developed any further.

I actually think it might be beneficial to use typed-ast from pypi in the < 3.8 test, but that's a discussion for another PR.

@srittau
Copy link
Collaborator Author

srittau commented Sep 26, 2019

Concerning INSTALL: I don't find the indirection particularly appealing, especially since the only duplication is between the flake8 and pytest steps. And in a next step it would possible make sense to split the packages installed for CI, since installing pytest takes quite some time and is just unnecessary for the flake8 step.

@bluetech
Copy link
Contributor

the only duplication is between the flake8 and pytest steps

It also deduplicates the pip install -U git+git://github.com/python/mypy git+git://github.com/python/typed_ast part which is what I was thinking about.

@JelleZijlstra JelleZijlstra merged commit 23232c0 into python:master Sep 30, 2019
@srittau srittau deleted the simplify-travis-config branch September 30, 2019 10:10
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