Skip to content

feat(inputs): add support for attribute representation#27

Merged
vic merged 3 commits into
denful:mainfrom
HeitorAugustoLN:attribute-inputs
Oct 16, 2025
Merged

feat(inputs): add support for attribute representation#27
vic merged 3 commits into
denful:mainfrom
HeitorAugustoLN:attribute-inputs

Conversation

@HeitorAugustoLN
Copy link
Copy Markdown
Contributor

Adds support for all flake reference attributes to the flake-file.inputs option. This allows users to specify inputs using the attribute set representation, in addition to the URL-like syntax.

Copilot AI review requested due to automatic review settings August 6, 2025 21:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for attribute representation to flake reference inputs, allowing users to specify inputs using individual attribute options instead of just URL-like syntax. The change enhances the flexibility of input configuration by providing granular control over flake reference attributes.

  • Adds 10 new option attributes for flake references (type, owner, repo, path, id, dir, narHash, rev, ref, host)
  • Removes trailing whitespace from the module

type = lib.types.str;
};
type = lib.mkOption {
description = "type of flake reference";
Copy link

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

The description for 'type' option is too generic. Consider being more specific about what types are valid (e.g., 'git', 'github', 'gitlab', 'sourcehut', 'path', etc.) to help users understand the expected values.

Suggested change
description = "type of flake reference";
description = "Type of flake reference. Valid values: 'git', 'github', 'gitlab', 'sourcehut', 'path', etc.";

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I could use an enum instead of a str value here, what do you think @vic?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yep, using an enum is better since the valid values of type is limited and not an open ended value.

Something even better would be to make sure that other attributes make sense if type is specified. eg, if type is specified then url makes no sense? and only some of the attributes are valid for each corresponding type, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

if type is specified then url makes no sense?

They can be used in conjunction, for example, if type is git or file.

and only some of the attributes are valid for each corresponding type, right?

Yeah, path is only for path type. owner and repo only work in git forges types, etc.

Though, implementing this validation would be kinda tricky. But I'll try.

type = lib.types.str;
};
id = lib.mkOption {
description = "flake registry id";
Copy link

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

The description 'flake registry id' could be clearer. Consider expanding it to 'identifier for flake registry entries' or similar to better explain its purpose.

Suggested change
description = "flake registry id";
description = "Identifier for flake registry entries";

Copilot uses AI. Check for mistakes.
type = lib.types.str;
};
narHash = lib.mkOption {
description = "NAR hash of the flake";
Copy link

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

Consider expanding the NAR hash description to include the format or purpose, such as 'NAR hash of the flake for integrity verification' to help users understand when and why this would be used.

Suggested change
description = "NAR hash of the flake";
description = "NAR hash of the flake (e.g., 'sha256-...'), used for integrity verification to ensure the flake's contents have not been tampered with.";

Copilot uses AI. Check for mistakes.
HeitorAugustoLN and others added 3 commits August 6, 2025 21:34
Adds support for all flake reference attributes to the flake-file.inputs option. This allows users to specify inputs using the attribute set representation, in addition to the URL-like syntax.
@vic
Copy link
Copy Markdown
Member

vic commented Oct 16, 2025

@HeitorAugustoLN I'm merging this. Thank you!

@vic vic merged commit a90ed94 into denful:main Oct 16, 2025
6 checks passed
@HeitorAugustoLN HeitorAugustoLN deleted the attribute-inputs branch October 16, 2025 10:29
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