-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(eslint-plugin-internal): [no-dynamic-tests] new internal Lint rule to ban dynamic syntax in generating tests #11323
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
base: main
Are you sure you want to change the base?
Conversation
Thanks for the PR, @nayounsang! 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. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
View your CI Pipeline Execution ↗ for commit 987cd78.
☁️ Nx Cloud last updated this comment at |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11323 +/- ##
==========================================
+ Coverage 90.84% 90.85% +0.01%
==========================================
Files 501 502 +1
Lines 50911 51006 +95
Branches 8383 8412 +29
==========================================
+ Hits 46248 46343 +95
Misses 4648 4648
Partials 15 15
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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.
LGTM, thanks! Only the one request for an added test is blocking IMO. Everything else is nitpicks that can be ignored if you don't like them. 🙂
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!
PR Checklist
Overview
no-dynamic-tests
Ban these:
And add test cases for this feature
At this point, it seems too early to apply this rule directly to packages, so I'll just implement the logic.