Skip to content

treewide: replace attrs by formats or types.anything#1595

Merged
rycee merged 1 commit intonix-community:masterfrom
berbiche:replace-attrs-by-formats
Dec 1, 2020
Merged

treewide: replace attrs by formats or types.anything#1595
rycee merged 1 commit intonix-community:masterfrom
berbiche:replace-attrs-by-formats

Conversation

@berbiche
Copy link
Copy Markdown
Member

@berbiche berbiche commented Nov 6, 2020

Description

Replaces all instances of:

  • types.attrs with types.attrsOf types.anything where it made sense
  • types.attrs with pkgs.format.{json,yaml,ini} where it made sense

There are places where replacing types.attrs broke the evaluation and so wasn't done:

  • pam
  • home-environment
  • bash
  • zsh

Checklist

  • Change is backwards compatible.

  • [x ] Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

    • Added myself and the module files to .github/CODEOWNERS.

@berbiche
Copy link
Copy Markdown
Member Author

berbiche commented Nov 6, 2020

The build will fail because this hasn't been rebased on top of #1594

@berbiche
Copy link
Copy Markdown
Member Author

berbiche commented Nov 6, 2020

The tests case run much slower now (they use much much more memory). I'll have to investigate.

This has nothing to do with these changes.

@berbiche berbiche force-pushed the replace-attrs-by-formats branch from 434dfe6 to e868816 Compare November 15, 2020 23:07
@berbiche
Copy link
Copy Markdown
Member Author

@rycee it would be great if this could move forward as types.attrs is very annoying to deal with.

@rycee
Copy link
Copy Markdown
Member

rycee commented Nov 16, 2020

Thanks! Looks good I think, I just got confused about the commented line.

@rycee rycee force-pushed the replace-attrs-by-formats branch from e868816 to 461ce6c Compare November 17, 2020 22:17
@berbiche
Copy link
Copy Markdown
Member Author

@rycee I just now remembered reading about a module that gave importance to the ordering of keys in its JSON configuration.
Do you happen to know what module that was?
The formatting with jq actually reorders keys based on alphabetical ordering.

@rycee rycee force-pushed the replace-attrs-by-formats branch from 461ce6c to 3091cb4 Compare November 17, 2020 22:40
@rycee
Copy link
Copy Markdown
Member

rycee commented Nov 17, 2020

@berbiche I can't recall which one that might be but I don't think it is any of these since they all use types.attrs or types.attrsOf, which orders the keys.

@berbiche berbiche force-pushed the replace-attrs-by-formats branch from 3091cb4 to 44f9d68 Compare November 30, 2020 02:55
@rycee rycee merged commit 44f9d68 into nix-community:master Dec 1, 2020
@rycee
Copy link
Copy Markdown
Member

rycee commented Dec 1, 2020

Thanks! Merged to master, now that its clear that the test performance issue is unrelated.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants