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

rename require-tothrow-message to require-to-throw-message #296

Closed
chrisblossom opened this issue Jul 9, 2019 · 6 comments
Closed

rename require-tothrow-message to require-to-throw-message #296

chrisblossom opened this issue Jul 9, 2019 · 6 comments
Labels

Comments

@chrisblossom
Copy link
Contributor

@chrisblossom chrisblossom commented Jul 9, 2019

Would a PR be accepted that would rename require-tothrow-message to require-to-throw-message? tothrow is not a word and for those of us using a spell checker in our IDE (webstorm / vscode plugin) it is distracting.

I think the PR would not remove require-tothrow-message, but rename all instances of it and possibly deprecate the old name.

@jeysal
Copy link
Member

@jeysal jeysal commented Jul 10, 2019

I'm worried that it would be less clear that it refers to the toThrow matcher.
Questions upon reading the rule name might be:
Where does it "require to throw [a] message"? Why would I throw a message?

@chrisblossom
Copy link
Contributor Author

@chrisblossom chrisblossom commented Jul 11, 2019

Interesting, when I read it originally on the readme, I assumed it was a spelling mistake and wasn't didn't know exactly what it was referring to until I read the description. Which I could have interpreted it the same as you said as is.

I personally don't think that should/would be an issue, but it could be renamed to something like:

  • to-throw-require-message
  • require-message-in-to-throw

Also, this is inconsistent with other rules (that same concern should apply here too), see

current style tothrow style
prefer-to-have-length prefer-tohavelength
prefer-to-contain prefer-tocontain
prefer-to-be-undefined prefer-tobeundefined
prefer-to-be-null prefer-tobenull
prefer-strict-equal prefer-strictequal
prefer-spy-on prefer-spyon

I do not think the other rules should change. I think require-tothrow-message should be updated to match others.

@jeysal
Copy link
Member

@jeysal jeysal commented Jul 12, 2019

@chrisblossom
Copy link
Contributor Author

@chrisblossom chrisblossom commented Jul 15, 2019

Not sure if the rename is important enough to warrant the breaking change though

I personally think it is.

EDIT: I don't think it is worth releasing a breaking change just because of this, but instead adding a deprecation notice to the current rule and adding the renamed rule. Then whenever a breaking change is released, remove the require-tothrow-message rule.

@jeysal
Copy link
Member

@jeysal jeysal commented Jul 15, 2019

@SimenB thoughts?

SimenB pushed a commit that referenced this issue Oct 27, 2019
…#306)

BREAKING CHANGE: Rename `require-tothrow-message` to `require-to-throw-message`

closes #296
SimenB pushed a commit that referenced this issue Oct 27, 2019
…#306)

BREAKING CHANGE: Rename `require-tothrow-message` to `require-to-throw-message`

closes #296
@github-actions
Copy link

@github-actions github-actions bot commented Oct 27, 2019

🎉 This issue has been resolved in version 23.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

No branches or pull requests

2 participants