Skip to content

Conversation

@lucperkins
Copy link
Member

@lucperkins lucperkins commented Aug 29, 2025

Without this default, evaluating a nix-darwin config that uses this module fails if determinate-nix.customSettings is unspecified. As we can reasonably expect many people to not use this, we should set a default of {}, which the module handles by not writing any nix.custom.conf.

@grahamc
Copy link
Member

grahamc commented Aug 29, 2025

Oh ouch. Thanks. Can we add an evaluating check to this to make sure we don't break their configs again?

@lucperkins
Copy link
Member Author

lucperkins commented Aug 29, 2025

@grahamc What do you mean by an evaluating check? Like an assertion?

@cole-h
Copy link
Member

cole-h commented Aug 29, 2025

Probably means something in CI here that prevents the PR from being merged if we regress this.

@lucperkins
Copy link
Member Author

I'm not sure there's a great way to do that without adding a nix-darwin dependency

@grahamc
Copy link
Member

grahamc commented Aug 29, 2025

We actually already do this exact test, we just need to include our module 🙈 -- can you please integrate https://github.com/DeterminateSystems/determinate/pull/123/files into your branch?

@lucperkins
Copy link
Member Author

lucperkins commented Aug 29, 2025

@grahamc Makes sense! I did that and also added a check that ensures that passing actual custom settings evaluates properly.

@lucperkins lucperkins enabled auto-merge August 29, 2025 16:56
@lucperkins lucperkins merged commit 547096c into main Aug 29, 2025
9 checks passed
@lucperkins lucperkins deleted the custom-settings-default branch August 29, 2025 17:04
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.

4 participants