-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(eslint-plugin): add extension rule keyword-spacing
#1739
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
feat(eslint-plugin): add extension rule keyword-spacing
#1739
Conversation
Thanks for the PR, @OoDeLally! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day. |
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.
I've just done a basic lookover, focusing on your comments to help unblock you until someone can do a proper review :)
Happy to help if you have questions!
@G-Rath Ok, thanks for your comments. I'm now importing the base rule, it simplifies a bit. |
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.
Looking good!
Now that you're using the base rule, you can also drop the majority of the tests, since we don't need to re-test what eslint has already tested 😉
I'm now re-declaring the list of keywords both in typing and in string array. |
What is the deal with |
How is that |
You were missing a
If you click through to the details on the codecov site, it'll show you what branches of your code have not been hit by tests. Scrolling through that report, green means all good, yellow means one of the branches was missed, and red means all of the branches were missed. |
Would you know a way to avoid having to declare the option typing twice (in |
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.
a few comments, you might be able to simplify this by being a bit hacky - see my last comment.
Codecov Report
@@ Coverage Diff @@
## master #1739 +/- ##
=======================================
Coverage 94.37% 94.38%
=======================================
Files 164 165 +1
Lines 7572 7585 +13
Branches 2175 2175
=======================================
+ Hits 7146 7159 +13
Misses 183 183
Partials 243 243
|
This seems to fail every test with:
The error is in
I don't really understand what is going on, so I wouldn't risk replacing it. If you find a way to make it work, I'll be happy to simplify it. |
Co-Authored-By: Brad Zacher <brad.zacher@gmail.com>
Co-Authored-By: Brad Zacher <brad.zacher@gmail.com>
Co-Authored-By: Brad Zacher <brad.zacher@gmail.com>
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.
Thanks!
keyword-spacing
Addressing my own issue
fixes #1729
I am totally new to this, so feel free to suggest me improvements.