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

Package y-py #17076

Merged
merged 13 commits into from
Mar 14, 2022
Merged

Package y-py #17076

merged 13 commits into from
Mar 14, 2022

Conversation

davidbrochart
Copy link
Member

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@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/y-crdt, recipes/y-py) and found it was in an excellent condition.

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

recipes/y-py/build.sh Outdated Show resolved Hide resolved
recipes/y-py/meta.yaml Outdated Show resolved Hide resolved
recipes/y-py/build.sh Outdated Show resolved Hide resolved
Copy link
Member

@isuruf isuruf left a comment

Choose a reason for hiding this comment

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

Send a PR to update the dev branch of rust-feedstock
Also you need to package the copyright notices/license files of downstream packages

@bollwyvl
Copy link
Contributor

Here's another maturin build that needed an update to rust:

https://github.com/conda-forge/oxigraph-feedstock/blob/master/recipe/meta.yaml

recipes/y-py/meta.yaml Outdated Show resolved Hide resolved
recipes/y-py/meta.yaml Outdated Show resolved Hide resolved
@davidbrochart davidbrochart marked this pull request as ready for review December 21, 2021 11:01
version: {{ version }}

source:
url: https://github.com/yjs/y-crdt/archive/v{{ version }}.tar.gz
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be lovely if the upstream on pypi were used, just so we have some chain for the automation to follow. tags are great... but can be moved...

Copy link
Contributor

Choose a reason for hiding this comment

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

... of course, it would need the source anyway as the pre-compiled wheel isn't going to do much, but it does demonstrate that it is "well-packaged" upstream, has the licenses in the canonical distribution, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking about that too, but the PyPI package is a bit behind: https://pypi.org/project/y-py/#files.


test:
imports:
- y_py
Copy link
Contributor

Choose a reason for hiding this comment

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

it has no deps today, but having pip check can be very helpful for if/when it does grow some other deps, and is generally good to help keep things running over time.

recipes/y-py/meta.yaml Outdated Show resolved Hide resolved
@davidbrochart
Copy link
Member Author

OK to merge?

@SylvainCorlay
Copy link
Member

I think we should use the PyPI source package now that it is available.

@davidbrochart
Copy link
Member Author

We can't, the PyPI source package doesn't ship the Rust source code, only the y-py directory, which points to the absent directories above. Also, it doesn't ship the LICENSE which is one directory above.

@davidbrochart
Copy link
Member Author

I'm not sure the THIRDPARTY.yml generated license file is enough. I opened y-crdt/ypy#2 to add a LICENSE file to the PyPI package.

@davidbrochart
Copy link
Member Author

davidbrochart commented Feb 8, 2022

I build the recipe locally, and in the info/licenses/THIRDPARTY.yml file we have:

  - package_name: yrs
    package_version: 0.2.1
    license: MIT
    licenses:
      - license: MIT
        text: NOT FOUND

@dmonad @Horusiath could it be that the license is not packaged in https://github.com/y-crdt/y-crdt?
The version doesn't seem right either (should be 0.2.2).
Also I think we still need a license for y-py too (see y-crdt/ypy#2).

This was referenced Feb 14, 2022
recipes/y-py/meta.yaml Outdated Show resolved Hide resolved
Co-authored-by: Isuru Fernando <[email protected]>
@davidbrochart
Copy link
Member Author

@Waidhoferj @dmonad @Horusiath if you want to also be maintainers of this package, please post a comment confirming you are willing to, and I'll add you to the maintainer section.

@SylvainCorlay
Copy link
Member

🎉

@Waidhoferj
Copy link

@davidbrochart I'd love to participate as a maintainer.

@dmonad
Copy link

dmonad commented Mar 14, 2022

Thank you for getting this in @davidbrochart!

I think you and @Waidhoferj should be listed as maintainers.

@davidbrochart
Copy link
Member Author

Done in conda-forge/y-py-feedstock#2.

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.

8 participants