Skip to content
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: use url.Values for Qualifiers #57

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tommyknows
Copy link
Contributor

BREAKING CHANGE: This commit removes all the custom qualifier-logic that existed in order to keep the ordering of the qualifiers. The spec says:

sort this list of qualifier strings lexicographically

However, it doesn't say anything about the ordering within the typed representation.

Using url.Values through a type alias gives users an easier way to access specific values and removes code that we now don't need to maintain anymore.

See #53 for more information.

#53 briefly touches on the release process as well. Because this library is still in alpha-release (v0.x.y), I don't think we need to bump the major version. Releasing a v0.2.0 would be fine according to the SemVer rules.

I'm aware that this is quite a big change to the API, but I think better now than later :)

@shibumi
Copy link
Collaborator

shibumi commented Aug 12, 2023

Releasing a v0.2.0 would break our current release scheme. We map the minor version to the current purl spec version (probably not the best way and we should aim for a v1 instead.. I know).

BREAKING CHANGE: This commit removes all the custom qualifier-logic that
existed in order to keep the ordering of the qualifiers. The spec says:

> sort this list of qualifier strings lexicographically

However, it doesn't say anything about the ordering within the typed
representation.

Using `url.Values` through a type alias gives users an easier way to
access specific values and removes code that we now don't need to
maintain anymore.

See package-url#53 for more information.
@tommyknows
Copy link
Contributor Author

Okay I've just rebased this PR. I'm not sure what to do with the versioning scheme / releases, so I'll leave it to you to merge this PR or close it!

@shibumi
Copy link
Collaborator

shibumi commented Aug 13, 2023

We could cut a v1.0.0 before merging this and then bump the major version to v1.1.0, but I am still not 100% happy about this.

Mirroring Spec releases to the same version is very impractical, it might be better to create spec directories like: v1/packageurl.go and if a v2 drops we have a v2/packageurl.go. 🤔 This way we could uncouple the versions.

@shibumi
Copy link
Collaborator

shibumi commented Aug 13, 2023

I will open a separate issue for this and ask for some feedback from the other maintainers :)

@shibumi shibumi mentioned this pull request Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants