-
Notifications
You must be signed in to change notification settings - Fork 204
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
Update MissingPackage
error to allow reporting by package name
#19972
Conversation
e8b34a3
to
e11d5ef
Compare
@@ -86,7 +86,7 @@ final class ValueEnricher( | |||
|
|||
private[this] def handleLookup[X](lookup: => Either[LookupError, X]) = lookup match { | |||
case Right(value) => ResultDone(value) | |||
case Left(LookupError.MissingPackage(pkgId, context)) => | |||
case Left(LookupError.MissingPackage(PackageRef.Id(pkgId), context)) => |
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.
Case where we need different handling for missing package by id vs name, and can distinguish easily enough.
final case class PackageWithName(packageName: PackageName) extends Reference { | ||
override def pretty: String = s"package $packageName" |
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.
Replaced with Package(PackageRef.Name(...))
def apply(packageRef: Ref.PackageRef): MissingPackage = | ||
MissingPackage(packageRef, language.Reference.Package(packageRef)) | ||
|
||
def apply(packageId: Ref.PackageId): MissingPackage = apply(Ref.PackageRef.Id(packageId)) |
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.
Keep overloads for building with a PackageId
for convenience and compatibility.
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.
Looks reasonable.
Thanks.
e11d5ef
to
e5a1e3c
Compare
e5a1e3c
to
33fdc75
Compare
In the participant, the handling of a command which contains a template id whose package id is not known should be the same, irrespective of whether the template id specified the package with a package id or a package name. From that point of view, it makes sense to handle both cases with the same error.
If the
MissingPackage
type contained aPackageRef
rather than aPackageId
it could make this more uniform, and cases which did need to distinguish could still pattern match on the subtype ofPackageRef
, as needed.