-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
V2.0.0 #14
V2.0.0 #14
Conversation
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 ( |
@pelson I guess that the tarball is missing something, no? |
Change looks good. Something is well borked though. I'll double check what's going on... |
The PyPI tarball is mising the |
Confirming I get the same thing locally 😞 |
Let's go for cf_units v2.0.1 😉 |
Windows is still failing but I have a hunch that is due |
recipe/build.sh
Outdated
@@ -1,9 +1,7 @@ | |||
#!/bin/bash | |||
|
|||
# Make sure cf_units can find the udunits library. | |||
SITECFG=cf_units/etc/site.cfg |
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 recon we can remove the site.cfg file for Linux & OSX.
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 left it there for "symmetry" no strong opinions and glad to remove it.
Setting the environment variable UDUNITS2_XML_PATH should have an impact on whether the tests pass. This wouldn't normally be necessary, but there is a bug with the UDUINTS build, as described at conda-forge/udunits2-feedstock#18. Don't think we need to hold up this PR to fix that issue, if we can get the tests passing using the UDUNITS2_XML_PATH env var (with a comment referencing the issue, so that we know when we can remove the restriction). |
Edit: I never committed the |
Working on a version of |
set SITECFG=cf_units/etc/site.cfg | ||
echo [System] > %SITECFG% | ||
echo udunits2_path = %SCRIPTS%\udunits2.dll >> %SITECFG% | ||
echo udunits2_xml_path = %LIBRARY_PREFIX%\share\udunits\udunits2.xml >> %SITECFG% |
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.
Personally, I'd have kept this, as it helps cf_units work even if users don't activate their environment.
That said, I'm not overly concerned about it 😄
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.
Makes sense. I can re-add that in another PR later. I guess I was confused due to your comment on this line for Linux and OS X before. Should I re-add build.sh
and bld.bat
with the cf_units/etc/site.cfg
and the udunits2.xml
path on both?
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.
Sorry, confusing messages. conda does the binary replacement just fine for OSX and Linux, just not for Windows. Happy to have the config file for all platforms though for consistency.
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.
No problem.
Awesome stuff @ocefpaf!! |
This will probably fail and we'll need to work on it a little bit to smooth out the corners.
Ping @pelson