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

Add dsnparse feedstock #6606

Merged
merged 8 commits into from Sep 10, 2018

Conversation

astrojuanlu
Copy link
Member

Created directly with conda skeleton and minimum editing.

@conda-forge-linter
Copy link

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 (recipes/dsnparse) and found some lint.

Here's what I've got...

For recipes/dsnparse:

  • The recipe must have some tests.

@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 (recipes/dsnparse) and found it was in an excellent condition.

@astrojuanlu
Copy link
Member Author

I tried patching MANIFEST.in to include the license file but apparently it's not enough. Before I send a pull request upstream, can someone confirm that this is the right way to go?

@synapticarbors
Copy link
Member

The MANIFEST.in determines what goes in the tarball (the sdist) that goes on pypi. Patching will have no effect since by the time you fetch the tarball it already hasn't been included. You should put in a pull request in the upstream repo and just manually package the license_file here:

https://conda-forge.org/docs/meta.html#packaging-the-license-manually

@@ -0,0 +1,20 @@
From 7ab57006a38816c13a9ef37a5df7aad63b136459 Mon Sep 17 00:00:00 2001
Copy link
Member

Choose a reason for hiding this comment

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

Delete this patch and manually package the license file:

https://conda-forge.org/docs/meta.html#packaging-the-license-manually

url: https://pypi.io/packages/source/{{ name[0] }}/{{ name }}/{{ name }}-{{ version }}.tar.gz
sha256: 6252d2a0b12e7c5bf1775e77ed0d3ee0a06f92937cdb2d2cac6ee039b557d0b4
patches:
- 0001-Include-License-in-source-distribution.patch
Copy link
Member

Choose a reason for hiding this comment

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

remove the patch.

- 0001-Include-License-in-source-distribution.patch

build:
number: 0
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this could be noarch: python

https://conda-forge.org/docs/meta.html#building-noarch-packages

@astrojuanlu
Copy link
Member Author

Thanks @synapticarbors! Tracking here: Jaymon/dsnparse#5

@astrojuanlu
Copy link
Member Author

@conda-forge-admin, please add noarch: python

@astrojuanlu
Copy link
Member Author

Error seems unrelated, will retry tomorrow

@synapticarbors
Copy link
Member

Just a note, I don't think you can request noarch from the webservices from staged-recipes. I think it only works from a feedstock. I've just added it manually myself.

@astrojuanlu
Copy link
Member Author

From the docs you sent 😃

or you can just ask the webservice to add it for you and rerender: say @conda-forge-admin, please add noarch: python in an open PR.

Anyway, thanks! It's still failing with the same error though

@astrojuanlu astrojuanlu closed this Sep 6, 2018
@astrojuanlu astrojuanlu reopened this Sep 6, 2018
@astrojuanlu
Copy link
Member Author

There's an upstream issue: conda/conda-build#3114

@drnextgis
Copy link
Contributor

It looks like an upstream issue was fixed.

@marcelotrevisani
Copy link
Member

LGTM!

Thanks to put this in conda-forge! :)

@marcelotrevisani marcelotrevisani merged commit 12a03c5 into conda-forge:master Sep 10, 2018
@astrojuanlu astrojuanlu deleted the dsnparse-feedstock branch February 4, 2020 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants