-
Notifications
You must be signed in to change notification settings - Fork 206
Fixes for bitbucket #627
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
base: main
Are you sure you want to change the base?
Fixes for bitbucket #627
Conversation
Signed-off-by: Prabhu Subramanian <[email protected]>
| }, | ||
| "version_definition": { | ||
| "note": "The version is a commit or tag.", | ||
| "requirement": "required", |
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.
It is unnecessarily restricting the usefulness of PURL to require a version 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.
Agreed, @prabhu let's not make versions required
| "version_definition": { | ||
| "note": "The version is a commit or tag.", | ||
| "requirement": "required", | ||
| "note": "The version is an immutable commit hash (full or short) or a tag. Branch names (e.g., 'main') should not be used as they are mutable pointers.", |
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.
This doesn't work. Tags are also mutable. Software that works with PURLs cannot reasonably knowing whether a reference is intended to be mutable.
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.
Makes sense. The original wording is simple enough and does not specify too little nor too much IMHO
| "pkg:bitbucket/birkenfeld/pygments-main@244fd47e07d1014f0aed9c", | ||
| "pkg:bitbucket/atlassian/[email protected]", | ||
| "pkg:bitbucket/atlassian/[email protected]#packages/analytics", |
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.
None of these examples exist? Where did these new examples come from? The Pygments one might date back to when Bitbucket still supported Mercurial.
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 point, the pygments example should go since the project long moved to GitHub from Mercurial. ... for other examples too, let's use something real
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.
@prabhu Thanks. Here are a few nitpickings for your consideration!
| "is_case_sensitve": false | ||
| "case_sensitive": false, | ||
| "normalization_rules": [ | ||
| "It is not case sensitive and must be lowercased." |
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.
This is redundant with case_sensitive, so we would not add this IMHO
| "native_name": "user, organization, or project key", | ||
| "case_sensitive": false, | ||
| "normalization_rules": [ | ||
| "It is not case sensitive and must be lowercased." |
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_sensitive implies this alright
| "version_definition": { | ||
| "note": "The version is a commit or tag.", | ||
| "requirement": "required", | ||
| "note": "The version is an immutable commit hash (full or short) or a tag. Branch names (e.g., 'main') should not be used as they are mutable pointers.", |
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.
Makes sense. The original wording is simple enough and does not specify too little nor too much IMHO
| "pkg:bitbucket/birkenfeld/pygments-main@244fd47e07d1014f0aed9c", | ||
| "pkg:bitbucket/atlassian/[email protected]", | ||
| "pkg:bitbucket/atlassian/[email protected]#packages/analytics", |
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 point, the pygments example should go since the project long moved to GitHub from Mercurial. ... for other examples too, let's use something real
|
Needs to be rebased as of #640 |
Fixes from gemini
is_case_sensitvehas been corrected to the standardcase_sensitivein both the namespace_definition and name_definition.