Skip to content

Conversation

@avnik
Copy link
Contributor

@avnik avnik commented Nov 9, 2023

No description provided.

@avnik avnik marked this pull request as draft November 9, 2023 15:15
Copy link
Member

@aciceri aciceri left a comment

Choose a reason for hiding this comment

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

I haven't tried it anyway. Does it already work?

Comment on lines 14 to 18
pkgs = import inputs.nixpkgs {
inherit system;
overlays = [
];
};
Copy link
Member

Choose a reason for hiding this comment

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

You can override _module.args.pkgs. IMO it's a bit cleaner than using let.

Comment on lines 24 to 28
checks = mkOption {
type = types.attrsOf types.package;
default = {};
internal = true;
};
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this? Can't we directly assigned to flake.checks?

type = types.lazyAttrsOf (types.submodule {
options = {
systems = mkOption {
type = types.listOf types.str;
Copy link
Member

Choose a reason for hiding this comment

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

If you want to create a cleaner type consider using lib.systems.flakeExposed which gives you a list of all systems used in nixpkgs.

lib/default.nix Outdated
Comment on lines 4 to 6
options.cardanoNix = lib.mkOption {
type = lib.types.lazyAttrsOf lib.types.submodule;
};
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed? Can't we simply declare options.cardanoNix.<thing>?

module = mkOption {
type = types.defferedModule;
};
};
Copy link
Member

Choose a reason for hiding this comment

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

This lacks a name option that by default is equal to config._module.args.name.

@aciceri
Copy link
Member

aciceri commented Nov 13, 2023

@avnik is there any reason why it's marked as draft or it's ready?

@aciceri aciceri marked this pull request as ready for review November 15, 2023 11:33
@aciceri aciceri merged commit 201d12c into master Nov 15, 2023
@aciceri aciceri deleted the avnik/tests branch November 15, 2023 13:25
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