Skip to content

futures now install_requires - #4991

Merged
theacodes merged 2 commits into
googleapis:masterfrom
david-gang:patch-futures-dependency
Mar 22, 2018
Merged

futures now install_requires#4991
theacodes merged 2 commits into
googleapis:masterfrom
david-gang:patch-futures-dependency

Conversation

@david-gang

@david-gang david-gang commented Mar 6, 2018

Copy link
Copy Markdown
Contributor

According to this comment the current extras_require format to specify platform dependencies is deprecated, so moving it upwards
Also look here at the documentation and at SO

According to pypa/pipenv#1547 (comment) the current extras_require format to specify platform dependencies is deprecated, so moving it upwards
Also look here:http://setuptools.readthedocs.io/en/latest/setuptools.html#declaring-platform-specific-dependencies
@david-gang
david-gang requested a review from theacodes as a code owner March 6, 2018 13:48
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 6, 2018
@theacodes

Copy link
Copy Markdown
Contributor

Interesting. I'll need to do some testing to make sure this doesn't break in some older toolchains, but thank you for bringing this to our attention!

@david-gang

Copy link
Copy Markdown
Contributor Author

Thanks.
As i can see in the code you require a recent version of setuptools anyway:
https://github.com/david-gang/google-cloud-python/blob/fa7b4ed8e099bf5ed0606e6f9196b0c0cb07f94f/api_core/setup.py#L36

@uranusjr

uranusjr commented Mar 7, 2018

Copy link
Copy Markdown

Here’s how Pytest makes sure about the environment marker support level, for the most paranoid:

https://github.com/pytest-dev/pytest/blob/6553468/setup.py#L26
https://github.com/pytest-dev/pytest/blob/6553468/setup.py#L68

In general most packages shouldn’t worry about this, unless with good reasons (I can imagine Google Cloud Platform being one of them). Keeping unnecessary support to the old syntax gives users incentive to use outdated versions of Setuptools, which has security implications.

@lukesneeringer

Copy link
Copy Markdown
Contributor

@jonparrott Any objection to my merging this?

@theacodes

Copy link
Copy Markdown
Contributor

@lukesneeringer I wanted to give this a test with some older pips.

@theacodes
theacodes merged commit cd8a69b into googleapis:master Mar 22, 2018
@theacodes

Copy link
Copy Markdown
Contributor

@david-gang thanks for doing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants