-
Notifications
You must be signed in to change notification settings - Fork 237
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
feat(rules): add no-jasmine-globals
#116
Conversation
oh. this is so awesome! |
Looks like coverage has dropped slightly and is breaking CI. Maybe we should add coveralls to this project? |
Made |
LGTM. 👏 |
@SimenB can we add a link to the docs for the recommended approach for the patterns we cannot auto fix? Edit: or put an example in the error message? |
@mattphillips good idea! Feel free to push some examples to the docs 🙂 |
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.
Looks good. Wonder if we should use stricter language, something stronger than "Avoid using"?
Changed "avoid using" to "illegal usage of" |
Like it 👍 |
when green :) |
@mattphillips PR welcome with more concrete examples for the docs 🙂 |
👍 |
🎉 This PR is included in version 21.16.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
@aaronabramov We should turn this on for WWW once you are done. |
Should this get added to the |
Of course! Maybe we can even make it recommended? |
I'll open a PR. |
I'll wait on making it recommended since @SimenB said:
|
Found a bug btw, when applying it to the jest code base. Pushed fix: 885ce17 |
🎉 This PR is included in version 21.16.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Interesting that the bot picked up I referenced the commit in this PR, so posted a comment. Probably a bug :P |
For posterity, these are the errors reported on the Jest code base with this rule active:
|
In preparation for jest-circus, warning users about Jasmine usage would be great.
Will probably add to recommended in next major (whenever eslint 5 comes out)
/cc @captbaritone