-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
Haskell: haskell.lib.composable init and switch #142940
Haskell: haskell.lib.composable init and switch #142940
Conversation
I think my main suggestion would be to split this into at least two pull requests:
|
Also take into account
|
This will make the nesting problem cause of the flipped arguments less problematic. |
I think that we all agree that this is the right way to go. The question is can we go in this direction and if so, how? Infinitely maintaining two versions of the library would be very unsatisfying. |
I don’t think it’s so big an annoyance that breaking all backwards compat is justified, and maybe not even big enough to split the ecosystem even further? I don’t see any good way to solve this initial misdesign now that so much code depends on it :( |
Well in a world where |
Done: #144705
Was this referring to the use of |
3ba7d3b
to
9d6968b
Compare
hahah, I just noticed that edit, this is actually explained in the commit message for pipe:
|
9d6968b
to
0ecc307
Compare
well, the previous functions are still available under I suppose I was definitely aiming for replacing the old set when putting the deprecation warning in, but I'd be very happy to compromise that away. |
0ecc307
to
6079de0
Compare
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’m okay with the change in general, but I have some changes I would like to see:
- don’t deprecate the old module functions
- call it
.compose
instead ofcomposable
(just less typing and same content) - Define everything in
compose
in terms of the old definitions, don’t duplicate any implementation as it does right now (single source of truth). - Maybe add some documentation to the non-compose module header and the nixpkgs haskell documentation that
compose
is preferred.
Other than that I’m looking forward to using these functions in the future.
7b23391
to
bca5918
Compare
done done done, NixOS/cabal2nix#528
The old
Me too! |
Ah, I didn’t see that. It has the disadvantage that the history gets a bit mangled, but personally I don’t care which way around it is. |
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 next step will be to update https://haskell4nix.readthedocs.io/ to use compose in all examples. But due to the fact that it’s not “in tune” with the rest of the manual, that can’t be done in this repository.
No changes in derivations for pkgs.haskellPackages
bca5918
to
03a4d2f
Compare
03a4d2f
to
15ae25f
Compare
The derivation for the GHCJS `vector` package broke in #142940 due to introducing the line of code that this change deletes. The offending line appears to have been unintentionally added and causes an evaluation failure for two separate reason : * The argument order is wrong The change in #142940 switched the `haskellLib` utilities to flip their argument order, but the `appendPatch` in the offending line has the original argument order * The patch file referenced by the offending line does not exist The correct fix is to delete the line, because the patch is not necessary. The default version of the `vector` package is `0.12.3.1`, which already includes the fix from that patch.
Supersedes #100732
Basically, makes the
drv
argument to everything inhaskell.lib
the last argument. Some discussion in the above PR