-
Notifications
You must be signed in to change notification settings - Fork 4
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
staging: patch r-sleuth NAMESPACE #11
Conversation
bimsb/packages/staging.scm
Outdated
(substitute* "NAMESPACE" | ||
(("^importFrom\\(rhdf5,h5write\\.default\\)" line) | ||
(string-append "#" line))) | ||
#t))))) |
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.
FYI: we no longer need #T at the end of build phases. On the master
branch it still prints a warning, but on core-updates-frozen (to be merged today, hopefully) it's all fine.
Long ago we used booleans to indicate success or failure for build phases. Then we switched to using exceptions for failures. With core-updates-frozen
the build systems are finally fully aware of this change.
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.
Could you please add a link to the upstream discussion above this build phase. It's always good to keep a record right in the code.
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 for the feedback. Much appreciated. I tried to address both points. Please double-check though since I did not test actually building sleuth
using this latest iteration. 😉
* Drop superfluous import of function not exported by rhdf5 anymore.
38b67cb
to
eab626d
Compare
I'm confused by the discussion around r-sleuth. Is this change a workaround for an upstream problem or a fix? Will sleuth not only build fine but also be usable? IIUC upstream essentially abandoned sleuth. If that is so, perhaps we should just remove the package. Or is it worth keeping? Sorry for being so dense. (It's heavy bones.) |
Hope this helps, sorry for not being dense at all (as usual). 😉 |
Thank you, this sounds all reasonable. I've applied and pushed your patch. Thanks! |
In the meantime the upstream bug has been fixed. So with the next version bump, these changes should be undone. |
This is a workaround for pachterlab/sleuth#259 (effectively applying pachterlab/sleuth#260 after unpacking the package sources).
Without it,
r-sleuth
cannot be installed with Guix.