-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
Conversation
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 ( |
@@ -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'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fun. 😕
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok ^^
So this is set to go into |
Ok, added an |
@@ -4,3 +4,8 @@ travis: | |||
appveyor: | |||
secure: | |||
BINSTAR_TOKEN: MP4hZYylDyUWEsrt3u3cod2sbFeRwUziH02mvQOdbjsTO/l1yIxDkP/76rSIjcGC | |||
channels: | |||
sources: | |||
- conda-forge |
There was a problem hiding this comment.
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.
Couple comments. Should be easy to address. Otherwise LGTM. |
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. |
Ok, thanks. Noted. |
@jakirkham so would people that subscribed to the matplotlib rc also get this? |
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 |
There was a problem hiding this comment.
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.
Yep, unfortunately. Though it is better than putting it in There was some discussion of having a flag for 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.
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.
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. |
Arlgl:
|
Sooo... disable the tests ? lol |
There are other options. Will look at it more when I'm back in front of a computer (on phone now). |
No description provided.