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 zppy + zstash to conda-forge #21883

Merged
merged 24 commits into from
Feb 8, 2023
Merged

Conversation

mahf708
Copy link
Contributor

@mahf708 mahf708 commented Jan 27, 2023

Need to fix:

  • licenses
  • python noarch, etc.
  • lint

--

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.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • 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-webservices
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/zppy, recipes/zstash) and found some lint.

Here's what I've got...

For recipes/zppy:

  • The home item is expected in the about section.
  • The summary item is expected in the about section.

For recipes/zstash:

  • The home item is expected in the about section.
  • The summary item is expected in the about section.
  • noarch: python recipes are required to have a lower bound on the python version. Typically this means putting python >=3.6 in both host and run but you should check upstream for the package's Python compatibility.

@xylar
Copy link
Contributor

xylar commented Jan 27, 2023

@mahf708, please also add @tomvothecoder, @forsyth2 and @chengzhuzhang.

Tom, Jill and Ryan, please post here verifying that you agree to be mainainers.

@conda-forge-webservices
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/zppy, recipes/zstash) and found some lint.

Here's what I've got...

For recipes/zppy:

  • The home item is expected in the about section.
  • The summary item is expected in the about section.
  • There are too few lines. There should be one empty line at the end of the file.
  • Recipe maintainer "chengzhuzhang." does not exist

For recipes/zstash:

  • The home item is expected in the about section.
  • The summary item is expected in the about section.
  • There are too few lines. There should be one empty line at the end of the file.
  • Recipe maintainer "chengzhuzhang." does not exist
  • noarch: python recipes are required to have a lower bound on the python version. Typically this means putting python >=3.6 in both host and run but you should check upstream for the package's Python compatibility.

@conda-forge-webservices
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/zppy, recipes/zstash) and found some lint.

Here's what I've got...

For recipes/zppy:

  • There are too few lines. There should be one empty line at the end of the file.
  • Recipe maintainer "chengzhuzhang." does not exist

For recipes/zstash:

  • There are too few lines. There should be one empty line at the end of the file.
  • Recipe maintainer "chengzhuzhang." does not exist
  • noarch: python recipes are required to have a lower bound on the python version. Typically this means putting python >=3.6 in both host and run but you should check upstream for the package's Python compatibility.

@conda-forge-webservices
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/zppy, recipes/zstash) and found some lint.

Here's what I've got...

For recipes/zppy:

  • There are too few lines. There should be one empty line at the end of the file.

For recipes/zstash:

  • There are too few lines. There should be one empty line at the end of the file.
  • noarch: python recipes are required to have a lower bound on the python version. Typically this means putting python >=3.6 in both host and run but you should check upstream for the package's Python compatibility.

@conda-forge-webservices
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/zppy, recipes/zstash) and found some lint.

Here's what I've got...

For recipes/zstash:

  • noarch: python recipes are required to have a lower bound on the python version. Typically this means putting python >=3.6 in both host and run but you should check upstream for the package's Python compatibility.

@conda-forge-webservices
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/zppy, recipes/zstash) and found it was in an excellent condition.

@mahf708
Copy link
Contributor Author

mahf708 commented Jan 27, 2023

I fixed all the obvious linting errors. I will have a closer look upstream to ensure we are doing this correctly. This will still likely fail

@forsyth2
Copy link

please post here verifying that you agree to be mainainers.

I agree to be a maintainer.

Other questions:

@mahf708
Copy link
Contributor Author

mahf708 commented Jan 27, 2023

  • I'm seeing "Only those with write access to this repository can mark a draft pull request as ready for review." Do I need write access to this repository or is it sufficient for others to have that access?

You will get access to the repos (they will be named zppy-feedstock and zstash-feedstock) once this PR is merged. I can add you to this PR to edit, if you'd like, but there is no need. We will discuss what to do about the "upstream" tools/actions separately (I will need to consult with Xylar about what's best to do)

recipes/zstash/meta.yaml Outdated Show resolved Hide resolved
@mahf708
Copy link
Contributor Author

mahf708 commented Jan 27, 2023

Thanks @xylar, I just copied the recipes from upstream. Please take another look when you get a chance

@xylar
Copy link
Contributor

xylar commented Jan 27, 2023

@mahf708 and @forsyth2, the Linux checks are failing because of the discrepancy I mentioned in E3SM-Project/zstash#201 (comment). We will need to get this figured out before we can proceed.

@xylar
Copy link
Contributor

xylar commented Jan 27, 2023

@mahf708, it seems like maybe things are going to need to be fixed upstream so setup.py is correct and pip check can pass. We could turn off pip check for now with a note to turn it back on after the next release. What do you think?

@xylar
Copy link
Contributor

xylar commented Jan 27, 2023

@mahf708 mahf708 marked this pull request as ready for review January 28, 2023 16:51
@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-webservice.

I was asked to ping @conda-forge/help-python and so here I am doing that.

@xylar
Copy link
Contributor

xylar commented Jan 31, 2023

@mahf708, I don't think there's any need to merge the main branch into this branch. There should never be any conflicts. Each recipe is in its own directory.

@mahf708
Copy link
Contributor Author

mahf708 commented Jan 31, 2023

You're right... a bad subconscious habit 🤷

@xylar
Copy link
Contributor

xylar commented Feb 1, 2023

@mahf708, I don't think it hurts to have asked for a review because it can take awhile but we're still resolving:
E3SM-Project/zppy#395
and
E3SM-Project/zstash#250

It's important that a reviewer doesn't merge this until those have been resolved.

@mahf708 mahf708 marked this pull request as draft February 1, 2023 02:39
@mahf708
Copy link
Contributor Author

mahf708 commented Feb 1, 2023

Ah sorry, I thought we were more or less done with the PRs. My bad. Let's wait until we are 100% sure and we will move forward then

recipes/zstash/meta.yaml Outdated Show resolved Hide resolved
recipes/zstash/meta.yaml Outdated Show resolved Hide resolved
mahf708 and others added 3 commits February 2, 2023 17:09
Co-authored-by: Xylar Asay-Davis <xylarstorm@gmail.com>
Co-authored-by: Xylar Asay-Davis <xylarstorm@gmail.com>
Co-authored-by: Xylar Asay-Davis <xylarstorm@gmail.com>
@mahf708
Copy link
Contributor Author

mahf708 commented Feb 3, 2023

@xylar are we good to go? Should I make this PR ready for review?

@xylar
Copy link
Contributor

xylar commented Feb 3, 2023

Okay, looks good. Yes, I think this is ready for review.

@mahf708 mahf708 marked this pull request as ready for review February 3, 2023 03:17
@mahf708
Copy link
Contributor Author

mahf708 commented Feb 3, 2023

@conda-forge-admin, please ping conda-forge/help-python, ready for review, thanks!

These two packages are mostly for HPC usage for the E3SM project, and we thought it would be better to house them on conda-forge here than private channels. Thank you! The Windows failure is expected, these are mostly linux-64 packages and we do not guarantee anything beyond that.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-webservice.

I was asked to ping @conda-forge/help-python and so here I am doing that.

@mahf708
Copy link
Contributor Author

mahf708 commented Feb 7, 2023

@xylar could you nudge the team? I cannot do it because this is a new account.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-webservice.

I was asked to ping @conda-forge/help-python, and so here I am doing that.

@xylar
Copy link
Contributor

xylar commented Feb 8, 2023

@mahf708, I prefer not to. They are busy and this isn't urgent. I only nudge them when it's urgent and that makes it more likely that my nudges are taken seriously.

@dopplershift
Copy link
Member

Ignoring noarch Windows build failure due to missing dependency.

@dopplershift dopplershift merged commit 0617e3f into conda-forge:main Feb 8, 2023
@xylar
Copy link
Contributor

xylar commented Feb 8, 2023

@dopplershift, much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

6 participants