Proposal: Changing some linting rules, making some fixes#8295
Conversation
jobelenus
requested review from
jkeiser,
vbustamante,
wendybujalski and
zacharyhamm
Dependency Review✅ No vulnerabilities or OpenSSF Scorecard issues found.Scanned FilesNone |
| if (connected) wsConnections.value[workspaceId] = connected; | ||
|
|
||
| await db.setConnections({ ..._wsConnections.value }); | ||
| db.setConnections({ ..._wsConnections.value }); |
There was a problem hiding this comment.
I like that we don't await non-async things! Is this enforced by require-await? Seems like a hidden feature of that rule.
|
|
||
| // warning on this because we have some shenanigans where it is a promise but a literal `await` is not present | ||
| "@typescript-eslint/require-await": "warn", | ||
| "@typescript-eslint/require-await": 0, |
There was a problem hiding this comment.
This change is the only one I'm curious about. We don't want this to be "error" (2)?
There was a problem hiding this comment.
Both in the pinia stores, and in the web worker ... basically, we know that with Comlink, things return a promise, so its an async function, even if there isn't an await inside of it... and the same goes to the pinia store... there isn't an await, but the funcs have to be async for the plugin that we wrote to work
There was a problem hiding this comment.
I think 0 is better than warn right now, so I'm going to approve. But for future discussion, what do you think about using explicit ignores where we are forced to do this? (And can you use await anyway, or does that actually change the behavior? Thinking it might remain the same.)
jobelenus
force-pushed
the
jobelenus/linting-rules
branch
from
93f7648 to
e564b24
Compare
Proposal: Changing some linting rules, making some fixes
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
jobelenus
force-pushed
the
jobelenus/linting-rules
branch
from
e564b24 to
3ab1291
Compare
jobelenus
force-pushed
the
jobelenus/linting-rules
branch
from
3ab1291 to
58c6351
Compare
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
jobelenus
force-pushed
the
jobelenus/linting-rules
branch
from
58c6351 to
247bcd3
Compare
5 participants
Proposal for some linting rule fixes—DISCUSS.
This basically leave one rule in warning state that is giving useful feedback to us while making decisions
@typescript-eslint/restrict-template-expressions