-
Notifications
You must be signed in to change notification settings - Fork 764
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
Refactor: cleanup usage of parseTwoPartID #313
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.
👍 on the tradeoff between end user experience and developer friction.
This function is used in many resources to parse colon-separated two-part identifier strings, and it emits an error message when the string does not match the expected format. The error message indicates that the format was expected to be "organization:name" but this is only true for some resources, not all of them. As a result users may get an error while attempting to import a resource which gives them incorrect information about the identitier format. This patch improves the function to accept a pair of label strings, and it uses them when constructing the error message, so the message will be tailored to the call site's desired format for the string. Signed-off-by: Kevin P. Fleming <kpfleming@bloomberg.net>
The docstring should refer to the function's argument names and they were changed from "a, b" to "left, right". Signed-off-by: Kevin P. Fleming <kpfleming@bloomberg.net>
This one triggered 3 failed tests:
I've got this queued for investigation but looks more involved than the others. |
Hmm, I've run the tests locally instead of through CI and am seeing positive results. I feel confident in merging this change to unblock #318. |
…rtid Refactor: cleanup usage of parseTwoPartID
This function is used in many resources to parse colon-separated
two-part identifier strings, and it emits an error message when
the string does not match the expected format. The error message
indicates that the format was expected to be "organization:name"
but this is only true for some resources, not all of them. As a
result users may get an error while attempting to import a resource
which gives them incorrect information about the identitier format.
This patch improves the function to accept a pair of label strings,
and it uses them when constructing the error message, so the message
will be tailored to the call site's desired format for the string.
Signed-off-by: Kevin P. Fleming kpfleming@bloomberg.net