Skip to content

lib: introduce imap0, imap1 #25543

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 4, 2017
Merged

lib: introduce imap0, imap1 #25543

merged 3 commits into from
Jul 4, 2017
+31 −17

Conversation

zimbatm
Copy link
Member

@zimbatm zimbatm commented May 5, 2017

Motivation for this change

For historical reasons, imap starts counting at 1 and it's not
consistent with the rest of the lib.

So for now we split imap into imap0 that starts counting at zero and
imap1 that starts counting at 1. And imap is marked as deprecated.

See c71e2d4#commitcomment-21873221

Also:

$ ag "imap " | wc -l
115
Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

@zimbatm
lib: introduce imap0, imap1

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
fe89ba0 
For historical reasons, imap starts counting at 1 and it's not
consistent with the rest of the lib.

So for now we split imap into imap0 that starts counting at zero and
imap1 that starts counting at 1. And imap is marked as deprecated.

See NixOS@c71e2d4#commitcomment-21873221
@mention-bot
Copy link

@zimbatm, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @Profpatsch and @nbp to be potential reviewers.

@zimbatm zimbatm requested review from aszlig, edolstra and nbp May 5, 2017 19:23
@zimbatm
Copy link
Member Author

zimbatm commented May 5, 2017 •
edited
Loading

If I get 👍 for this PR I will also start replacing all the imap occurences

zimbatm referenced this pull request May 5, 2017
@zimbatm
lib/lists: document all functions

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
c71e2d4
@Profpatsch
Copy link
Member

I thought about maybe having imapN n f xs, but that’s probably a bad idea because that’s trivially just n: f: imap0 (f n) and complicates the function.
The original imap name is probably gone for good, because of the semantics change (without a type change). So yeah, +1 from me.

@zimbatm
Copy link
Member Author

zimbatm commented May 6, 2017

In lib/strings.nix we also have concatImapStrings and concatImapStringsSep that start counting at 1

@zimbatm
Copy link
Member Author

zimbatm commented May 6, 2017

After replacing all the lib.imap calls there is only one instance where imap0 was used.

nbp
nbp approved these changes Jul 3, 2017
View reviewed changes
lib/lists.nix Outdated
But for consistency with the rest of the library we want an index
starting at zero.
*/
imap = imap1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: move it to lib/deprecated.nix

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 54a81c8

@zimbatm zimbatm merged commit 4d54529 into NixOS:master Jul 4, 2017
@zimbatm zimbatm deleted the lib-imap0 branch July 4, 2017 22:29
@mguentner
Copy link
Contributor

@zimbatm

Please fix pkgs/stdenv/booter.nix:
https://github.com/NixOS/nixpkgs/blob/master/pkgs/stdenv/booter.nix#L76

@NeQuissimus
Copy link
Member

@mguentner @zimbatm I fixed booter.nix in d788244 since I was just building something off master and got lots of error: attribute ‘imap’ missing, at /home/nequi/dev/nixpkgs/pkgs/stdenv/booter.nix:76:30 :)

NeQuissimus added a commit that referenced this pull request Jul 5, 2017
@NeQuissimus
stdenv / booter: imap -> imap1
d788244 
Fix the usage of imap as a follow-up to #25543
@zimbatm
Copy link
Member Author

zimbatm commented Jul 5, 2017

thanks @NeQuissimus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nbp nbp

@0xABAB 0xABAB

@aszlig aszlig

@edolstra edolstra

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@zimbatm @mention-bot @Profpatsch @mguentner @NeQuissimus @nbp @0xABAB