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.

Sign up for GitHub

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

Jump to bottom

Add py37 linux #3254

Merged
merged 11 commits into from
Aug 22, 2018
Merged

Add py37 linux #3254

merged 11 commits into from
Aug 22, 2018

Conversation

lasote
Copy link
Contributor

@lasote lasote commented Jul 23, 2018 •
edited
Loading

Only for Linux, pushed docker image for conan testing with 3.7 installed.

@lasote lasote added this to the 1.7 milestone Jul 23, 2018
@ghost ghost assigned lasote Jul 23, 2018
@ghost ghost added the stage: review label Jul 23, 2018
@lasote
Copy link
Contributor Author

lasote commented Aug 21, 2018

@ytimenkov The linter plugin is failing with python 3.7 and I don't have a clue why. Apparently, when the pylint_plugin.py is loaded, it stops working general pylint checks like indentation or unused vars. In Macos is working good, but failing in Linux and Windows.

Any idea? Thanks!

@ytimenkov
Copy link
Contributor

My best guess is API change somewhere in Conan so when plugin tries to find type for the field in transform_conanfile, something throws an exception.
I will try to reproduce myself.

ytimenkov
ytimenkov reviewed Aug 21, 2018
View reviewed changes
conans/requirements.txt Outdated
future==0.16.0
pygments>=2.0, <3.0
astroid>=1.6, <1.7
astroid
Copy link
Contributor

Choose a reason for hiding this comment

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

Just thought: Conan doesn't depend on astroid directly (except for plugin). Does it still need to be specified in requirements or could we just let pip figure out what's needed?

Copy link
Contributor Author

@lasote lasote Aug 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

As the plugin is importing "astroid" we should declare it as requirement.

Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason to not require at least some minimum version? It is very likely that it will fail with older pylint and astroid versions

@lasote lasote requested a review from memsharded August 22, 2018 07:49
@lasote
Copy link
Contributor Author

lasote commented Aug 22, 2018

The CI is ok, @ytimenkov Many thanks for your help!

@lasote lasote assigned memsharded and unassigned lasote Aug 22, 2018
@ghost ghost assigned lasote Aug 22, 2018
@memsharded memsharded merged commit 8828a08 into conan-io:develop Aug 22, 2018
@ghost ghost removed the stage: review label Aug 22, 2018
@lasote lasote mentioned this pull request Aug 27, 2018
Closed
grisumbras pushed a commit to grisumbras/conan that referenced this pull request Dec 27, 2018
@lasote @memsharded
Add py37 linux (conan-io#3254)
e950c07 
* Add py37 linux

* update image

* Refresh

* Fixed images

* cryptography 2.3 required in mac for python 3.7

* fixing

* better range

* unlock astroid

* unlock astroid and pylint

* Minimum version for astroid and pylint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ytimenkov ytimenkov ytimenkov left review comments

@memsharded memsharded Awaiting requested review from memsharded

Assignees

@lasote lasote

@memsharded memsharded

Labels
None yet
Projects
None yet
Milestone
1.7
Development

Successfully merging this pull request may close these issues.
3 participants
@lasote @ytimenkov @memsharded