Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Some fixes for require-tothrow-message #161

Merged
merged 7 commits into from
Sep 30, 2018
Merged

Some fixes for require-tothrow-message #161

merged 7 commits into from
Sep 30, 2018

Conversation

garyking
Copy link
Contributor

@garyking garyking commented Sep 29, 2018

I allow for any argument type, which would include TemplateLiteral, CallExpression, etc.

I adjusted the tests by throwing actual errors, adding comments, adding more tests.

I couldn't add a test for TemplateLiteral since it requires babel-eslint.

I couldn't break up the commit into multiple commits because one of the precommit hooks staged and commited the entire file, even when I staged only a few lines at a time.

@SimenB
Copy link
Member

SimenB commented Sep 30, 2018

I couldn't add a test for TemplateLiteral since it requires babel-eslint

That doesn't make any sense. Would it be enough to bump ecmaVersion?

I couldn't break up the commit into multiple commits because one of the precommit hooks staged and commited the entire file, even when I staged only a few lines at a time.

What I usually do is just do --no-verify to skip the hook. Hopefully it'll be better in the future: lint-staged/lint-staged#75

// code:
// "const a = 'a'; expect(() => { throw new Error('a'); })" +
// '.toThrow(`${a}`);',
// parser: 'babel-eslint',
Copy link
Member

Choose a reason for hiding this comment

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

I think it should work if you set ecmaVersion: https://eslint.org/docs/user-guide/configuring#specifying-parser-options

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

Should activate the template literal test, other than that this LGTM!

@garyking
Copy link
Contributor Author

@SimenB Okay changes have been made.

@SimenB SimenB merged commit f2d2dbe into jest-community:master Sep 30, 2018
@SimenB
Copy link
Member

SimenB commented Sep 30, 2018

🎉 This PR is included in version 21.24.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

2 participants