Skip to content

komac: migrate to finalAttrs#387439

Merged
donovanglover merged 1 commit intoNixOS:masterfrom
HeitorAugustoLN:komac-finalattrs
Mar 7, 2025
Merged

komac: migrate to finalAttrs#387439
donovanglover merged 1 commit intoNixOS:masterfrom
HeitorAugustoLN:komac-finalattrs

Conversation

@HeitorAugustoLN
Copy link
Member

@HeitorAugustoLN HeitorAugustoLN commented Mar 5, 2025

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@HeitorAugustoLN HeitorAugustoLN added the 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. label Mar 5, 2025
@HeitorAugustoLN HeitorAugustoLN requested a review from kachick March 5, 2025 23:43
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Mar 5, 2025
Copy link
Member

@kachick kachick left a comment

Choose a reason for hiding this comment

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

I can understand the motivation for replacing rec with finalAttrs.
Is it also desirable to change from the current let-in style?

I don't know the details of the background. I just found #293452 and #194475. The issues seem to be under discussion.

@HeitorAugustoLN
Copy link
Member Author

I can understand the motivation for replacing rec with finalAttrs. Is it also desirable to change from the current let-in style?

I don't know the details of the background. I just found #293452 and #194475. The issues seem to be under discussion.

Using rec or let-in means that self-references won’t update if an attribute is overridden, which can lead to mismatches. With finalAttrs (recursive fixed-point), when you override something like version, every reference that uses it (such as in tag or changelog) updates automatically. This keeps everything consistent without needing manual overrides for each attribute.

Copy link
Member

@kachick kachick left a comment

Choose a reason for hiding this comment

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

Thank you for explaining. 🙏

@kachick kachick added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Mar 6, 2025
@donovanglover donovanglover merged commit 6b31915 into NixOS:master Mar 7, 2025
38 checks passed
@donovanglover donovanglover mentioned this pull request Mar 7, 2025
13 tasks
@HeitorAugustoLN HeitorAugustoLN deleted the komac-finalattrs branch March 7, 2025 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants