-
Notifications
You must be signed in to change notification settings - Fork 45
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
BREAKING CHANGE: Invalidate special chars #13
Conversation
index.js
Outdated
@@ -68,6 +68,10 @@ var validate = module.exports = function(name) { | |||
warnings.push("name can no longer contain capital letters") | |||
} | |||
|
|||
if (/[~'!()*]/.test(name.split('/').slice(-1)[0])) { | |||
warnings.push('name can no longer contain special characters ("~\'!()*")') |
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.
curious about the choice of using warn
rather than error, I imagine:
- on the registry we'll block new publications that cause this warning.
- the CLI will log the message but allow the publish.
- old packages will continue to be able to be published?
This seems reasonable to me, just refreshing myself with this module.
This seems to do what it says on the tin. Since it only affects new packages, I'm fine with this. Agreed that it is a breaking change. |
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.
lgtm
Changes Unknown when pulling 55f1162 on invalidate-special-chars into ** on master**. |
Merging and publishing as v3.0.0. |
No longer allow
~!'()*
in package names in any position (Surprise, they're url-safe!)The first commit contains the relevant change; the second commit upgrades the
builtins
&tap
deps; the final commit (noisily)standard
-formats the repo.