Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
feat(eslint-plugin): [prefer-nullish-coalescing] change ignoreConditionalTests default to true#8872
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…onalTests default to true
Thanks for the PR,@JoshuaKGoldberg! 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 onhttps://opencollective.com/typescript-eslint. |
netlifybot commentedApr 7, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Deploy Preview fortypescript-eslint ready!
To edit notification comments on pull requests, go to yourNetlify site configuration. |
nx-cloudbot commentedApr 7, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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. Yay!
Regarding the description, I don't see a reason to change the name of the option.... People that have already specified the option manually will get to keep their configuration, rather than being punished with a broken lint run just because we changed our minds about a default. And people who have not specified it will be impacted identically independently of whether the name is changed.
…onalTests default to true (#8872)
Uh oh!
There was an error while loading.Please reload this page.
BREAKING CHANGE:
Changes the default value of a rule's option.
PR Checklist
ignoreConditionalTests
totrue
#8243Overview
Straightforward change of the default value from
false
totrue
.I'd considered renaming the option to
checkConditionalTests
so we could avoid having a truthy default... but figured that was a bit more user effort than folks are likely to want. I'd be happy to make that change if requested though.💖