-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
V4.9.0 #140
V4.9.0 #140
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 ( |
Linux is failing with:
|
I'll take a look to see how to duplicate and then fix this, and will round out our CI as needed; this isn't occurring on our end as-is. Thanks. |
BTW, what version of zstd are you using in your CIs? |
The json code occurs twice in our code base: once for use by plugins exclusively |
The version being installed by the underlying package manager, so let me see if I can extract that information. |
@WardF @DennisHeimbigner Did you had any time to look into this? |
OK, after a bit of digging this is already in the works somehow: Unidata/netcdf-c#2419 |
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:
|
@ocefpaf I've added a patch for this to the PR branch. Let's see if this works. |
Hmm, the created patch fails to apply... |
Maybe it wasn't worth the try. Hopefully 4.9.1 is out anytime soon. |
…strange error. See if the error follows the patch or if it was tied to the file.
It looks like this is passing and ready for somebody else to review and merge. Thanks all! |
Thanks @WardF! Indeed it looks like a task that only someone with more intimated knowledge of the source code could fix. While I'm OK merging this, there is a lot of patching and I wonder if it makes sense to wait for a new release in case that is happening soon. |
@ocefpaf I wanted to have one out by the end of the month, and that could still happen; if it's not out this week, I'm going to try to have it out next week. This was the largest blocking issue to resolve, but I still have some documentation to add and/or revamp, among all the other sundy day-to-day items we all deal with XD. I don't have any particular insight into the workflow and/or implications of skipping this version in libnetcdf-feedstock, however, so I'll leave that for a broader group discussion :) |
Sorry, just getting caught up on the conversation. It sounds like it doesn't make sense to push anymore changes to the branch until we know if we're merging this or waiting for 4.9.1. |
Having thought about it, I believe this should be merged; I'll also ensure the patches contained in this (and, moving forward, other) PR's find their way back upstream into the |
With the exception of the ncjson patch (taken from upstream), all the patches added here tweak CMake/a few tests (and associated build files), so I'm comfortable that what we have here is fine and would be comfortable merging. @xylar do you want to make the changes to eliminate the static build? |
Thanks, @hmaarrfk! Those are almost exactly the commits I had staged. Except that I didn't think to check that the static library is not there, which is excellent! |
I think i got it, removing the static. |
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.
Assuming CI passes, I'm very happy to see this merged.
With all that said, we probably want to wait for the (soon-to-be-released) 4.9.1 to think about migrating downstream compiled packages. |
@dopplershift, I think that probably makes sense. Have this version available but not migrate to it? |
@xylar Yeah. We can always decide to migrate if for some reason 4.9.1 becomes delayed, but that shouldn't be the case. No need to do two migrations in rapid succession, but having it available can also allow for some more testing by interested users. |
Who dares hit the green button? |
Closes #139