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

Allow npm names to be case sensitive #73

Merged
merged 2 commits into from
Dec 15, 2024

Conversation

PFCM
Copy link
Contributor

@PFCM PFCM commented Sep 25, 2024

Package names on npm have not always been forced to lower case, it is only enforced for new packages (see https://docs.npmjs.com/cli/v10/configuring-npm/package-json).

There are a number of cases where there are two distinct packages that differ only in casing, such as (borrowing from package-url/packageurl-java#38):

https://www.npmjs.com/package/Base64/v/1.0.0

https://www.npmjs.com/package/base64/v/1.0.0

Notably the Java implementation already does this (PR linked above), this discrepancy is certainly unfortunate.

Copy link
Collaborator

@shibumi shibumi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM

@shibumi
Copy link
Collaborator

shibumi commented Nov 27, 2024

I think this makes absolutely sense. If we don't allow this, we will have no way to distinguish between older npm packages.

NPM is really a mess..

LGTM

@shibumi
Copy link
Collaborator

shibumi commented Nov 27, 2024

@PFCM can you retrigger a CI run via: git commit --amend && git push --force to the branch used in this PR? The CI system hangs :/ happens sometimes on GitHub

@shibumi
Copy link
Collaborator

shibumi commented Dec 15, 2024

Sorry for the delay, this PR is still blocked by: #76
Once #76 got merged, we can merge this one here, too

@shibumi shibumi merged commit 4b484f7 into package-url:master Dec 15, 2024
2 checks passed
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