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
lib.strings.intersperse
-> lib.lists.intersperse
.
#75102
Conversation
Please fix the re-export in https://github.com/NixOS/nixpkgs/blob/master/lib/default.nix#L81 too |
Done. |
Another issue has popped up. Let me run this one through the ringer until I have a passing build. |
42835c4
to
852a120
Compare
There we go. This branch is ready for merging. |
1b44fca
to
c1b3498
Compare
`intersperse` is defined within `lib.strings` yet does not work for sintersperse trings, only lists. Furthermore, it only returns a list. There is no sense in a function that only works with lists being in the strings library.
@@ -54,9 +54,10 @@ rec { | |||
separator: | |||
# Input list | |||
list: | |||
if list == [] || length list == 1 | |||
lib.warn "`lib.strings.intersperse' is deprecated and will be removed. Please use `lib.lists.intersperse'." |
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.
@d-goldin How is this?
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.
Maybe just call lib.lists.intersperse
directly here instead of duplicating the logic? A dependency on lib.lists already exists anyways below.
Edit: To clarify, I mean intersperse = lib.warn "deprecated" lib.lists.intersperse
directly - or did I miss something?
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.
Oh, good call, that was a moment of stupidity. I'll fix that.
Thank you for your contributions. This has been automatically marked as stale because it has had no activity for 180 days. If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity. Here are suggestions that might help resolve this more quickly:
|
Closing this, since no reviewers with merge rights showed up. Sorry! |
intersperse
is defined withinlib.strings
yet does not work forstrings, only lists. Furthermore, it only returns a list. There is no
sense in a function that only works with lists being in the strings
library.
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @