Skip to content
This repository has been archived by the owner on May 21, 2022. It is now read-only.

Change error message casing #161

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

Conversation

e-dard
Copy link

@e-dard e-dard commented Aug 12, 2016

When updating to v3, one of our unit tests broke. Looking into it, I see at some point error messages were capitalised.

Error messages should not be capitalised unless they're acronyms or proper nouns, so that they can be composed with other text when, for example, logging them.

See here for more details.

@dgrijalva
Copy link
Owner

Thanks for catching this. Let me review the history of these strings and think about how this semver applies here.

The reason those errors are published is so you can compare against them directly. Is there a reason you don't use the constants in your tests directly?

@e-dard
Copy link
Author

e-dard commented Aug 12, 2016

The one that caught us out was the expiration one, which does not have an exported variable. I'm happy to add explicit variables in this PR, which would be a further improvement.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants