Conversation
c902790 to
6f14a84
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR removes the directional context parameters (fromUser, fromHost, OS, HM) from the aspect system, simplifying the API and reducing boilerplate. The changes streamline how aspects receive and propagate context by using direct parameter patterns instead of directional indicators.
- Refactored core parametric functions to use direct context passing instead of directional wrappers
- Updated aspect functors to explicitly take and pass context types (OS, HM)
- Simplified example code by removing unused directional context parameters
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/examples/modules/_example/ci/standalone-hm-special-args-osconfig.nix | Added ... parameter to accept additional context arguments |
| templates/examples/modules/_example/ci/one-os-package-per-user.nix | Removed OS and fromUser directional parameters and helper wrapper |
| templates/examples/modules/_example/ci/host-user-conditional-hm.nix | Removed HM directional parameter and helper wrapper, simplified conditional logic |
| templates/default/modules/aspects/eg/routes.nix | Simplified routing implementation by removing directional wrappers and using direct context application |
| nix/lib.nix | Reversed merge order in parametric.expands from attrs // ctx to ctx // attrs |
| modules/aspects/provides/user-shell.nix | Replaced exactly wrappers with direct parameter patterns and explicit parametric.atLeast |
| modules/aspects/provides/home-manager.nix | Updated to pass HM and host directly in context instead of nested directional structure |
| modules/aspects/dependencies.nix | Major refactor splitting dependencies into separate OS and HM paths without directional context indicators |
| modules/aspects/definition.nix | Updated aspect functors to explicitly take context types (HM/OS) and use new parametric patterns |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #84.
This PR removes the directional contexts
fromHostandfromUser. Now people can include funcitons like:And values are not duplicated now. Added test based on report from #84.
This also simplified a lot the number of contexts we have. Still have to update documentation about contexts.