-
Notifications
You must be signed in to change notification settings - Fork 66
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
Improve invalid package name errors #92
Conversation
}; | ||
} | ||
|
||
return true; |
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.
Oh rad. One suggestion - could return multiple errors if the package name fails on multiple rules?
yes. yes i could. |
these are all of the possible generated values - any copy suggestions?
|
if (length) { | ||
|
||
if (length > 1) { | ||
errors[length - 1] = 'and must ' + errors[length - 1]; |
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.
Should we loop through errors
? If there are 3 errors, this only modifies the 2nd one but not the 3rd one.
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.
nevermind i just reviewed the copy, it makes sense
Thanks! copy looks good. overall lgtm. |
@desandro - you good? |
thumbs up |
Improve invalid package name errors
deployed, thanks @patrickkettner @desandro |
Yay for communicative error messages! nice one @patrickkettner |
Notcied a number of issues cropping up that boiled down to how bad the error message is, this attempts to fix that.