-
Notifications
You must be signed in to change notification settings - Fork 48
Define pkg
as a PackageURL class attribute
#184
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
@jaimergp this is fine by me BTW... no objections to make it easier to reuse! |
Ah, I see, this is now with #186 |
Ah, @pombredanne, if you are fine with this type of change, yes, please, let's reopen! |
@pombredanne (or anyone with merge rights!) gentle ping for a hopefully uncontroversial review! |
src/packageurl/__init__.py
Outdated
@@ -314,6 +314,8 @@ class PackageURL( | |||
https://github.com/package-url/purl-spec | |||
""" | |||
|
|||
SCHEME: str = "pkg" |
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.
I'm wondering if using lower-caps scheme
for consistency with existing vars would make more sense. I don't see anything declared as upper-caps in that module.
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.
I used uppercase because it's supposed to be constant for the class, maybe I can indicate that with ClassVar[str]
instead.
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.
Changed to ClassVar[str]
Thanks @tdruez, let me know if I addressed your concerns! |
@jaimergp Thanks! LGTM. @pombredanne Could you give it another look and take care of the merge? |
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 fine! Thanks!
This will allow library users to subclass
PackageURL
and redefine the scheme to create PURL-like derivatives. (e.g. for thedep:
idea described here):Which can be used in the same way and happily accepts PEP440-like version ranges in the
version
field 🥳: