@metamask/eslint-config*@6.0.0, yarn lint:fix #102
Conversation
rekmarks
force-pushed
the
@metamask/eslint-config@6.0.0
branch
from
d7babad
to
5bfd9fe
Compare
|
This is ready to go. |
rekmarks
force-pushed
the
@metamask/eslint-config@6.0.0
branch
from
23d7f99
to
9b9f741
Compare
rekmarks
force-pushed
the
@metamask/eslint-config@6.0.0
branch
from
9b9f741
to
408913f
Compare
| ManifestWalletProperty, | ||
| { | ||
| port: number; | ||
| dist: string; | ||
| outfileName: string; | ||
| }, | ||
| ]) { | ||
| ] { |
There was a problem hiding this comment.
not related to this PR, but having this type inline is confusing to read with the formatting
There was a problem hiding this comment.
If we lave this comment unresolved I'll get back to it!
| @@ -138,19 +163,22 @@ export async function buildWeb3Wallet(argv: YargsArgs): Promise<[ | |||
|
|
|||
| return endWeb3Wallet(); | |||
There was a problem hiding this comment.
looks like this function is used before its defined
There was a problem hiding this comment.
The lint config permits hoisting for functions declared with the function keyword, and we do use that pattern in a variety of places throughout the codebase.
There was a problem hiding this comment.
just confusing to read because it's used before its defined and returning like that reads as if it has unreachable code.
There was a problem hiding this comment.
Since the order of the functions in this file predates / is orthogonal to the lint config, let's leave this unresolved and follow up on it in a separate issue or PR as well!
| @@ -190,9 +212,13 @@ export async function manifest(argv: YargsArgs): Promise<void> { | |||
| } | |||
|
|
|||
| if (isInvalid) { | |||
| throw new Error(`Manifest Error: package.json validation failed, please see above errors.`); | |||
| throw new Error( | |||
| `Manifest Error: package.json validation failed, please see above errors.`, | |||
There was a problem hiding this comment.
should we still use backtick or ' if a string has no embedded expressions?
There was a problem hiding this comment.
I'm inclined to permit it if the lint config permits it, which it does. I think it's configurable. I'm not sure what we should prefer 🤔
Captured here: MetaMask/eslint-config#174
| verboseErrors: false, | ||
| v: false, | ||
| '_': ['init'], | ||
| 'verboseErrors': false, |
There was a problem hiding this comment.
can i vote again to remove this quote keys thing lol
There was a problem hiding this comment.
You can create an issue on https://github.com/MetaMask/eslint-config 😜
There was a problem hiding this comment.
captured here: MetaMask/eslint-config#175
@metamask/eslint-config*@6.0.0packages and peer dependencies.eslintrc.jsjsonESLint pluginyarn lint:fixI had to run
yarn lint:fixagain in the latest commit due to the rebase. Aside frompackage.jsonand.eslintrc.js, no manual edits were made to any files.