-
Notifications
You must be signed in to change notification settings - Fork 710
Qualified constraints (issue #3502) part 2 #4228
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
Conversation
I also removed 'unqualified' from PackagePath.hs and replaced all uses of it with 'scopeToplevel'. The meaning is identical, however 'scopeToplevel' is a less confusing name.
Prefixed all constructors of Qualifier with 'Qual' to make it easier to grep them. Renamed 'Unqualified' to 'Toplevel'.
/cc @grayjay |
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, modulo one comment.
@@ -150,7 +150,7 @@ projectFreezeConstraints plan = | |||
versionConstraints :: Map PackageName [(UserConstraint, ConstraintSource)] | |||
versionConstraints = | |||
Map.mapWithKey | |||
(\p v -> [(UserConstraint UserUnqualified p (PackagePropertyVersion v), | |||
(\p v -> [(UserConstraint UserToplevel p (PackagePropertyVersion v), |
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.
That's a lot clearer. Thanks for renaming the constructors.
++ "either a version range, 'installed', 'source' or flags" | ||
"expected a (possibly qualified) package name followed by a " ++ | ||
"constraint, which is either a version range, 'installed', " ++ | ||
"'source', 'test', 'bench', or flags" |
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.
Good catch!
@@ -361,7 +361,7 @@ dontUpgradeNonUpgradeablePackages params = | |||
where | |||
extraConstraints = | |||
[ LabeledPackageConstraint | |||
(PackageConstraint (unqualified pkgname) PackagePropertyInstalled) | |||
(PackageConstraint (scopeToplevel pkgname) PackagePropertyInstalled) |
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.
The non-upgradable package constraints can use ScopeAnyQualifier
now.
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.
agreed
This is some further refactoring to address issues discussed in PR #4219