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
Add recipe for OCW #1606
Add recipe for OCW #1606
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 ( |
@agoodm Can you please help me fix the failing build? |
Nice work @jarifibrahim
|
@jarifibrahim Thank you for getting this done so soon! |
@agoodm I see these packages on anaconda The following are from the
In the above snippet python 3 is used for building conda recipe.
Is there any way to build ocw conda package only for python 2? The ocw is not compatible with python 3 at this moment. |
@jarifibrahim I was referring to the main anaconda channel and the conda-forge channel. Those packages are available on my own channel as well, but I believe they need to at least be on the conda-forge channel in order to not cause a CI build failure. As for the python3 question I am not exactly sure at the moment myself. Someone in the conda-forge org might know. |
@agoodm Right. That means we'll have to add |
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.
Is this necessary? I don't recall seeing other recipe feedstocks containing standalone LICENSE files.
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.
@jarifibrahim Please read the attached review.
Thanks,m
# KIND, either express or implied. See the License for the | ||
# specific language governing permissions and limitations | ||
# under the License. | ||
|
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.
@jarifibrahim You'll need to get rid of all comments in the final staged recipe, including the license headers. You'll need to do the same for the other PRs you have made if you haven't done so already.
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.
Done.
|
||
source: | ||
git_url: https://github.com/apache/climate.git | ||
|
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.
This should probably be changed to point to PyPi, with md5 hash and all. Additionally leaving it like this will get the latest code from our master ocw branch rather than a stable release which is not what we want.
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.
Right.
# under the License. | ||
|
||
{% set name = "ocw" %} | ||
{% set version = "1.2.0" %} |
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.
Should be version 1.1.0, as 1.2.0 is not released yet.
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.
Done.
|
||
about: | ||
home: http://climate.apache.org/ | ||
license: Apache License |
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.
Should be "Apache License, Version 2.0"
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.
Done.
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipes/ocw:
|
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 ( |
|
||
about: | ||
home: http://climate.apache.org/ | ||
license: Apache License, Version 2.0 |
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 we at conda-forge prefer the shortened license form. So Apache 2.0
or similar.
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.
Please ask upstream to add the license file to MANIFEST.in
. Also please add an xref in a comment in the recipe so we can track this and update accordingly once the license is packaged in the sdist
. We are trying to ensure license files are included in the built packages.
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.
Also can optionally add license_family: Apache
.
|
||
extra: | ||
recipe-maintainers: | ||
- agoodm |
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.
Are you ok with being a maintainer of climate
at conda-forge, @agoodm?
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.
@jakirkham Yes I am.
build: | ||
skip: True # [py3k] | ||
number: 0 | ||
script: python setup.py install |
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.
Please add the arguments --single-version-externally-managed --record record.txt
. These are required when using setuptools
to make sure this gets packaged correctly.
|
||
about: | ||
home: http://climate.apache.org/ | ||
license: Apache License, Version 2.0 |
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.
Also can optionally add license_family: Apache
.
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.
Thanks for putting this together, @jarifibrahim.
A few comments to get this cleaned up. Noting some dependencies are not here yet, but I see they are being worked on other PRs. We can come back to this once they are packaged.
Are you taking this over somewhere else @agoodm ? Should this stay open? |
@jakirkham go ahead and close this. Since this PR is rather cluttered I think it would be best for me to start from a new one. |
Continued in PR ( #1784 ). |
No description provided.