-
Notifications
You must be signed in to change notification settings - Fork 213
Fix bazel tests #730
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 bazel tests #730
Conversation
Signed-off-by: Tushar Goel <[email protected]>
Signed-off-by: Tushar Goel <[email protected]>
|
@TG1999 One preliminary question I have: if the input PURL has an unencoded slash in the @mjherzog @pombredanne @matt-phylum @jkowalleck What do you think? |
|
If the input PURL has an unencoded slash, an exception should not be thrown. It is not part of the spec, and it breaks existing PURLs where slash is not encoded, including examples from previous versions of the spec and output from code that implements an older version of the spec. When I last updated the purl-survey output, 10 of 15 implementations passed tests that required slash to be unencoded in qualifier values. PURLs canonicalized by those implementations would be broken by turning unencoded slashes in qualifiers into errors. |
|
Thanks @matt-phylum . Putting aside slashes for a moment (which the submitted standard makes clear MUST be percent-encoded in a qualifiers value however we're meant to distinguish between MUST and SHOULD and MAY), if a |
pombredanne
left a comment
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.
Looking good now... I added a few touch ups based on @mjherzog reviews.
Signed-off-by: Philippe Ombredanne <[email protected]>
Signed-off-by: Philippe Ombredanne <[email protected]>
pombredanne
left a comment
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.
All clear now. Merging.
Reference:
Related: