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

Noarch package #33

Merged
merged 11 commits into from
Jul 5, 2022
Merged

Noarch package #33

merged 11 commits into from
Jul 5, 2022

Conversation

orbeckst
Copy link
Contributor

@orbeckst orbeckst commented Jun 28, 2022

Change mdanalysistests into a noarch package (see MDAnalysis/mdanalysis#3727)

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

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

I do have some suggestions for making it better though...

For recipe:

@orbeckst
Copy link
Contributor Author

@conda-forge-admin, please rerender

@orbeckst
Copy link
Contributor Author

Comments from PR #32

@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 (recipe) and found some lint.

Here's what I've got...

For recipe:

  • noarch packages can't have selectors. If the selectors are necessary, please remove noarch: python.
  • 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.

For recipe:

recipe/meta.yaml Outdated
@@ -12,16 +12,14 @@ source:
sha256: {{ sha256 }}

build:
number: 0
number: 1
noarch: python
script: python setup.py install --single-version-externally-managed --record record.txt
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can this be rewritten as

script: {{ PYTHON }} -m pip install . --no-deps -vv

(possibly change requirements.host setuptools --> pip)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no-deps will not work as we need MDA.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't that already defined under run?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, you're right... Do we need the --single-version-externally-managed --record record.txt options for something??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to use pip.

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

I do have some suggestions for making it better though...

For recipe:

@orbeckst
Copy link
Contributor Author

@conda-forge-admin, please rerender

@orbeckst
Copy link
Contributor Author

I'll probably close my manual attempt here and let conda-smithy do everything in PR #34 ...

@orbeckst orbeckst mentioned this pull request Jun 28, 2022
1 task
@orbeckst
Copy link
Contributor Author

The automated PR #35 really only added noarch but there was other stuff to fix. It looks as if this PR is just as good (and already has more things removed).

@IAlibay perhaps you could have a quick look when you have time?

@IAlibay
Copy link
Contributor

IAlibay commented Jun 28, 2022

The automated PR #35 really only added noarch but there was other stuff to fix. It looks as if this PR is just as good (and already has more things removed).

@IAlibay perhaps you could have a quick look when you have time?

👍🏽 I'll give it a look through in the morning.

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

Copy link
Contributor

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

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

This seems fine to me, I don't understand thing enough to be 100% sure, but the best thing we can do is test it out and see what happens.

@orbeckst orbeckst merged commit 900e686 into conda-forge:main Jul 5, 2022
@orbeckst orbeckst deleted the noarch-package branch July 5, 2022 21:56
@orbeckst orbeckst mentioned this pull request Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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