-
Notifications
You must be signed in to change notification settings - Fork 247
Fix project flags #253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix project flags #253
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, would just benefit from one additional comment documenting the mkOverride requirement.
@@ -9,7 +9,7 @@ let | |||
}); | |||
pkgSet = mkCabalProjectPkgSet { | |||
plan-pkgs = plan.pkgs; | |||
modules = [ { packages.buildable-test.flags.exclude-broken = true; } ]; | |||
modules = [ { packages.buildable-test.flags.exclude-broken = mkOverride 10 true; } ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should add a comment here why we need the mkOverride 10
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack
Project flags (from
stack.yaml
andplan.json
) are exported in amodules
attribute bystack-to-nix
andplan-to-nix
, but are not currently used. This change updatesmkStackPkgSet
andmkCabalProjectPkgSet
so that themodules
attribute is used (if present) and includes tests to check they are.