-
-
Notifications
You must be signed in to change notification settings - Fork 15
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
Make package noarch
#86
Conversation
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 recipe:
|
@conda-forge-admin, please rerender |
…nda-forge-pinning 2022.11.07.16.21.19
@conda-forge-admin, please rerender |
Hi! This is the friendly automated conda-forge-webservice. I tried to rerender for you, but it looks like there was nothing to do. This message was generated by GitHub actions workflow run https://github.com/conda-forge/spyder-kernels-feedstock/actions/runs/3414586633. |
recipe/meta.yaml
Outdated
- cloudpickle | ||
- ipykernel >=6.16.1,<7.0.0 | ||
- ipython >=7.31.1,<8.0.0 | ||
- jupyter_client >=7.3.1,<8.0.0 # [python_impl == 'pypy'] | ||
- jupyter_client >=7.3.4,<8.0.0 # [python_impl == 'cpython'] | ||
- jupyter_client >=7.3.4,<8.0.0 | ||
- pyzmq >=22.1.0 | ||
- wurlitzer >=1.0.3 # [not win] |
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 still not needed on Windows? Because it is noarch in conda-forge: https://github.com/conda-forge/wurlitzer-feedstock
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.
The thing is wurlitzer
is not really necessary on Windows because it can only do its magic only on Posix systems. That's why we don't require it there.
However, we have a method that prevents from loading the wurlitzer
IPython extension on Windows:
So I guess it's ok to have a noarch
package here? I think @mattip (for PyPy) and other people that want to have a Spyder-kernel in more esoteric platforms would be happy with this change, right?
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.
Would make it slightly more easy to maintain but doesn't really matter from that perspective, just my 2 cents. :)
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.
Just to clarify, the package is already noarch with this PR, the question is only if we can reduce it to one noarch package or need 2, the former would make maintenance slightly easier but it's no big deal.
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.
Two is already 9x better than the current 18 (for 2.4.0)
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 ( |
string: "unix_pyh{{ PKG_HASH }}_{{ PKG_BUILDNUM }}" # [unix] | ||
string: "win_pyh{{ PKG_HASH }}_{{ PKG_BUILDNUM }}" # [win] |
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.
What's this needed for? I haven't seen it before in conda recipes.
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.
I think it came from one of the iterations of a new PR to document how to make a package noarch: python
conda-forge/conda-forge.github.io#1839 but from the discussion there is not needed.
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.
I think it's still needed, only when conda/conda-build#4606 is released should it be fine.
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 a lot @BastianZim for your help with this! And @mattip for your review too!
This is a great improvement.
Checklist
0
(if the version changed)conda-smithy
(Use the phrase@conda-forge-admin, please rerender
in a comment in this PR for automated rerendering)Makes the package noarch.
Some more info: conda-forge/conda-forge.github.io#1839