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.

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

trying to build 0.18rc with rc tag on conda-forge channel #6

Closed
wants to merge 4 commits into from

Conversation

amueller
Copy link
Contributor

No description provided.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@@ -33,12 +33,12 @@ install:
bash $MINICONDA_FILE -b

export PATH=/Users/travis/miniconda3/bin:$PATH
conda config --add channels conda-forge
conda config --add channels ['conda-forge']
Copy link
Member

Choose a reason for hiding this comment

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

That's fun. 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok ^^

@jakirkham
Copy link
Member

So this is set to go into master currently. Am going to add a new branch for the rc. We will need to change the base branch and resolve conflicts potentially, but it shouldn't be bad.

@jakirkham jakirkham changed the base branch from master to rc September 14, 2016 17:29
@jakirkham
Copy link
Member

Ok, added an rc branch and changed this over to it. Now we should be good to go.

@@ -4,3 +4,8 @@ travis:
appveyor:
secure:
BINSTAR_TOKEN: MP4hZYylDyUWEsrt3u3cod2sbFeRwUziH02mvQOdbjsTO/l1yIxDkP/76rSIjcGC
channels:
sources:
- conda-forge
Copy link
Member

Choose a reason for hiding this comment

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

So I think we can just drop sources. This is default anyways.

@jakirkham
Copy link
Member

Couple comments. Should be easy to address. Otherwise LGTM.

@ogrisel
Copy link
Contributor

ogrisel commented Sep 14, 2016

note that 0.18rc has a broken test under Python 3 that has been fixed in 0.18rc1. FYI I am working on 0.18rc2 with another important bugfix so please do not publish the package for 0.18rc or 0.18rc1.

@jakirkham
Copy link
Member

Ok, thanks. Noted.

@amueller
Copy link
Contributor Author

@jakirkham so would people that subscribed to the matplotlib rc also get this?
Or do people usually only specify a particular package with a label?
If no-one gets this one without explicitly saying so, I feel like we shouldn't worry too much.

url: https://pypi.io/packages/source/s/scikit-learn/scikit-learn-{{ version }}.tar.gz
md5: a2f8b877e6d99b1ed737144f5a478dfc
url: https://github.com/scikit-learn/scikit-learn/archive/{{ version }}.tar.gz
md5: 3cb6b237bcfb77168fb0b41c4cb9cff0
Copy link
Member

Choose a reason for hiding this comment

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

Checksum changed. Though may not be worth bothering with until we have the RC we want.

@jakirkham
Copy link
Member

@jakirkham so would people that subscribed to the matplotlib rc also get this?

Yep, unfortunately. Though it is better than putting it in main.

There was some discussion of having a flag for conda to control this behavior. See this issue ( conda/conda#3125 ).

The CFEP (which is not yet accepted) suggests possibly including the package name in the label like so. This would resolve the problem, but is a bit verbose and feels a bit unfriendly if one starts installing lots of dev packages. Still it may be the best available option.

Not really sure which works best and all of them are a little incomplete. Would be curious to hear your thoughts/preferences.

Or do people usually only specify a particular package with a label?

Had suggested a mechanism for doing something like that as well, but it is a bit inelegant. It could also suffer from issues if a new dependency is added.

If no-one gets this one without explicitly saying so, I feel like we shouldn't worry too much.

Agreed. I'm not too worried about it either.

Simply having the user make the choice is the most important. The rest is questions about the granularity of that choice and its impact.

@ogrisel
Copy link
Contributor

ogrisel commented Sep 14, 2016

Arlgl:

.............................................................
command ./ci_support/run_docker_build.sh took more than 120 minutes to run

@amueller
Copy link
Contributor Author

Sooo... disable the tests ? lol

@amueller amueller closed this Sep 14, 2016
@jakirkham
Copy link
Member

There are other options. Will look at it more when I'm back in front of a computer (on phone now).

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.

4 participants