-
Notifications
You must be signed in to change notification settings - Fork 22
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
ESLint: error on missing lint ignore comments #7829
Conversation
still 102 problems remaining
values: serviceContext, | ||
}); | ||
} | ||
await Promise.all( |
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.
This is the only functional change in the PR. This should be safe (and more efficient)
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.
We also have an asyncForEach
helper so you don't have to use a map
that doesn't return anything
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 remember that helper, but it looks like it no longer exists?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7829 +/- ##
==========================================
- Coverage 72.63% 72.63% -0.01%
==========================================
Files 1274 1274
Lines 39886 39882 -4
Branches 7403 7404 +1
==========================================
- Hits 28973 28969 -4
Misses 10913 10913 ☔ View full report in Codecov by Sentry. |
.eslintrc.js
Outdated
"new-cap": [ | ||
"error", | ||
{ | ||
capIsNewExceptionPattern: "(TEST_|INTERNAL_|HACK_)", |
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.
💯
src/auth/authUtils.ts
Outdated
// https://github.com/pixiebrix/pixiebrix-extension/issues/7725 | ||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion | ||
// https://github.com/pixiebrix/pixiebrix-extension/issues/7725 | ||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion -- email is required |
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.
For !
comments, I'd expect to see an explanation of how we know it's non null
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 believe this one in particular will be fixed soon by @BLoe 's work.
src/analysis/analysisVisitors/varAnalysis/parseTemplateVariables.ts
Outdated
Show resolved
Hide resolved
// https://github.com/pixiebrix/pixiebrix-extension/issues/7725 | ||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion | ||
// https://github.com/pixiebrix/pixiebrix-extension/issues/7725 | ||
/* eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion |
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 always assumed "comment on top" was supported. It turns out it isn't yet :(
I think it's supported for @ts-*
comments though
containerElement.children[0].children[0].children[0] = listElement; | ||
|
||
const { container } = renderDocumentPreview(containerElement); | ||
|
||
// Select a dropdown inside a Col in List and open it | ||
await userEvent.click( | ||
// eslint-disable-next-line testing-library/no-container, testing-library/no-node-access | ||
// eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- see test's TODO comment |
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.
Nit: A single /* eslint-disable testing-library/no-container, testing-library/no-node-access */
might be best
No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack. Do not edit this comment manually. |
What does this PR do?
102 errors remaining