Skip to content
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

Add argument for default host attributes #19

Merged
merged 3 commits into from
Apr 8, 2021

Conversation

Pacman99
Copy link
Collaborator

@Pacman99 Pacman99 commented Apr 7, 2021

Generic argument to add defaults to all hosts. This could cover modules, extraArgs, system, and channelName. I would say keep some things, ex sharedModules, just for convenience and backwards-compatibility, but this is overall useful to prevent making the api surface too large. Whenever adding things to hosts, you no longer need to make a relevant default argument, users can just use defaultHostAttrs.

Example:

defaultHostAttrs = {
  modules = ...;
  channeName = ...;
  system = ...;
};

@Pacman99 Pacman99 changed the title Default profile attributes Add argument for default profile/host attributes Apr 7, 2021
@Pacman99 Pacman99 changed the base branch from master to staging April 7, 2021 19:12
@Pacman99 Pacman99 changed the title Add argument for default profile/host attributes Add argument for default profile attributes Apr 7, 2021
Copy link
Collaborator

@blaggacao blaggacao left a comment

Choose a reason for hiding this comment

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

Perfect & clean solution!

Please feel entitled to merge, if it's ready.

@blaggacao
Copy link
Collaborator

@gytis-ivaskevicius we where wondering if you wanted to preserve shared* top level flat api or deprecate it at some point in the future.

We can see how usage plays out and make that call whonce we get there.

@gytis-ivaskevicius
Copy link
Owner

I'd like to preserve shared* the API. It is a very nice and convenient way to deal with system configuration. One of the things that might be nice to do is some sort of separation like sharedDarwinModules and sharedNixOSModules but I am not sure about that.

Related: divnix/digga#232 (comment)

@Pacman99 Pacman99 changed the title Add argument for default profile attributes Add argument for default host attributes Apr 8, 2021
@Pacman99 Pacman99 merged commit faa7855 into gytis-ivaskevicius:staging Apr 8, 2021
blaggacao pushed a commit that referenced this pull request Apr 8, 2021
This reverts commit faa7855, reversing
changes made to 8a593ca.

```
  evalHostArgs =
    { channelName ? "nixpkgs"
    , modules ? []
    , system ? defaultSystem
    , extraArgs ? {}
    , ...
    }: defaultHostAttrs
      // { 
        inherit channelName system; 
        modules = sharedModules ++ modules;
        extraArgs = sharedExtraArgs // extraArgs;
      };

```

does not as intended.

- channelName would be "nixpkgs" even if a default is set.
blaggacao pushed a commit that referenced this pull request Apr 8, 2021
blaggacao pushed a commit that referenced this pull request Apr 9, 2021
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.

3 participants