Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
fix(eslint-plugin): [class-literal-property-style] allow getter when same key setter exists#8277
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
Thanks for the PR,@yeonjuan! 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 commentedJan 20, 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. |
Uh oh!
There was an error while loading.Please reload this page.
38a2136
to348267b
CompareThere 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.
Ithink there's an edge case that needs to be handled? But otherwise this looks good to me!
Uh oh!
There was an error while loading.Please reload this page.
7c28999
toa4cb56f
Compare…same key setter exists
e7e2b70
to819b411
Comparecodecovbot commentedFeb 1, 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.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## main #8277 +/- ##======================================= Coverage 87.71% 87.72% ======================================= Files 397 397 Lines 13963 13971 +8 Branches 4061 4065 +4 =======================================+ Hits 12248 12256 +8 Misses 1403 1403 Partials 312 312
Flags with carried forward coverage won't be shown.Click here to find out more.
|
Aside: could you please not force push PRs? It's a small thing, no worries, but fromhttps://typescript-eslint.io/contributing/pull-requests:
|
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 great, thanks! 🔥
Just one suggested added test. I can merge this in too to get it in quickly.
#8323 tracks adding a shared utility around the static names. Linking in this comment for tracking.
Uh oh!
There was an error while loading.Please reload this page.
Oh OK! Sorry about that. I was making the review difficult. 😅 |
No worries at all 😄 TIL I (re-)learned that |
yeonjuan commentedFeb 1, 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.
@JoshuaKGoldberg Oops, Maybe this rule should report error on this cases? // group: Record<string, ConfigOptionsType>group[category]??={heading:category,fields:[],}; |
I don't know how that comment got hidden - but, yes - could you take a look please actually? (I'm between things atm) |
yeonjuan commentedFeb 1, 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.
@JoshuaKGoldberg I made a pr for fixing it (#8336) |
…same key setter exists (typescript-eslint#8277)
Uh oh!
There was an error while loading.Please reload this page.
PR Checklist
Overview