Skip to content

Conversation

@hugovk
Copy link
Contributor

@hugovk hugovk commented Dec 15, 2017

And add python_requires to help pip.

Latest master fails on the old 2.7 on Appveyor: https://ci.appveyor.com/project/hugovk/cycler/build/1.0.1

@Kojoley
Copy link
Member

Kojoley commented Dec 15, 2017

Is PYTHON_VERSION and PYTHON_ARCH used anywhere?

@hugovk hugovk mentioned this pull request Dec 15, 2017
@hugovk
Copy link
Contributor Author

hugovk commented Dec 15, 2017

@Kojoley
Copy link
Member

Kojoley commented Dec 15, 2017

You are right... then a path you defined for 3.6 interfere with appveyor installed python.

@hugovk
Copy link
Contributor Author

hugovk commented Dec 15, 2017

Which line interferes?

At least AppVeyor passes: https://ci.appveyor.com/project/hugovk/cycler/build/1.0.4

By the way, should AppVeyor be enabled for this repo? Then you get builds for PRs on AppVeyor and Travis.

@Kojoley
Copy link
Member

Kojoley commented Dec 15, 2017

Which line interferes?

Appveyor has installed python under C:\Python36-x64 path https://www.appveyor.com/docs/build-environment/#python.
Actually I do not know why we are downloading and installing the python. It looks unnecessary to me.

By the way, should AppVeyor be enabled for this repo?

@tacaswell could you please enable appveyor on organization account for cycler? It looks like I can't do it myself.

@hugovk
Copy link
Contributor Author

hugovk commented Dec 15, 2017

I'm not sure Why Python is downloaded rather then using the existing versions, but I bet there was a good reason. Maybe a specific version was needed. Does @tacaswell know?

Should we try it out with the system Pythons?

@tacaswell tacaswell closed this Dec 15, 2017
@tacaswell tacaswell reopened this Dec 15, 2017
@codecov-io
Copy link

codecov-io commented Dec 15, 2017

Codecov Report

Merging #43 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #43   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines         177    177           
  Branches       50     50           
=====================================
  Hits          177    177

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eaa0f63...5dc55d8. Read the comment docs.

@tacaswell
Copy link
Member

power-cycled to see if appveyor will fire.

Those old build failures look like dependency issues (missing setuptools) rather than cycler not working with old versions of 2.7.

@tacaswell
Copy link
Member

ohh, sorry, not reading the urls clearly (@hugovk 's account above and the emails I got from appveyor were tied to my personal account not matplotlib's). I'll see if I can sort this out when not on tethered-to-phone internet.

@Kojoley
Copy link
Member

Kojoley commented Dec 15, 2017

Those old build failures look like dependency issues (missing setuptools) rather than cycler not working with old versions of 2.7.

Yeah, cycler uses python setup.py install, so setuptools should have been installed before calling it.

@hugovk
Copy link
Contributor Author

hugovk commented Dec 20, 2017

Is there anything I can do to move this closer to merge?

@tacaswell
Copy link
Member

https://ci.appveyor.com/project/matplotlib/cycler appveyor should be active now

@tacaswell tacaswell closed this Dec 20, 2017
@tacaswell tacaswell reopened this Dec 20, 2017
@tacaswell
Copy link
Member

'power cycled' again to trigger appveyor.

No good reason to not use the system python. This appevyor configuration file was cargo-cult-ed from matplotlib/matplotlib which needs to compile c/c++. It can probably be greatly simplified.

@Kojoley
Copy link
Member

Kojoley commented Dec 20, 2017

@hugovk I have rebased and forcepushed to your branch.

@tacaswell
Copy link
Member

I am quite happy that this is a -250 line PR

@tacaswell tacaswell merged commit 60f4da3 into matplotlib:master Dec 21, 2017
@tacaswell
Copy link
Member

Thanks @Kojoley and @hugovk !

@hugovk hugovk deleted the update-versions branch December 21, 2017 06:40
@QuLogic QuLogic added this to the v1.0 milestone Oct 25, 2021
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.

6 participants