Skip to content

Update @metamask/eslint-config to v4.1.0#9663

Merged
Gudahtt merged 2 commits intodevelopfrom
update-eslint-config
Oct 21, 2020
Merged

Update @metamask/eslint-config to v4.1.0#9663
Gudahtt merged 2 commits intodevelopfrom
update-eslint-config

Conversation

@Gudahtt
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt commented Oct 20, 2020

@metamask/eslint-config has been updated to v4.1.0. This update requires that we update eslint to v7 as well, which in turn requires updating most eslint-related packages.

@Gudahtt Gudahtt force-pushed the update-eslint-config branch 3 times, most recently from 6c0fd79 to e7173a2 Compare October 20, 2020 23:38
.eslintrc.js Outdated
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

node/shebang requires that the shebang is only used for scripts identified in the bin field of package.json. I think our development scripts should be exempt from this rule.

Copy link
Copy Markdown
Member Author

@Gudahtt Gudahtt Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These unreachable paths became detected with one of these package updates. Not entirely sure why.

@Gudahtt Gudahtt force-pushed the update-eslint-config branch from e7173a2 to e167e22 Compare October 20, 2020 23:54
@Gudahtt Gudahtt marked this pull request as ready for review October 21, 2020 00:14
@Gudahtt Gudahtt requested review from a team and kumavis as code owners October 21, 2020 00:14
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [e167e22]
Page Load Metrics (380 ± 46 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint309137136
domContentLoaded2486423789746
load2496433809746
domInteractive2476413789746

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Maybe we should rename this to something that isn't flagged as a callback? 🤔 I don't know if "callback" is the best description anyway.

It's more like a... wrapped function? Like "run this function in a specific environment". Callback implies to me that it's a way to signal when an async task has completed, which this is not.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was required due to node/no-callback-literal

@Gudahtt Gudahtt changed the title Update @metamask/eslint-config to v4 Update @metamask/eslint-config to v4.1.0 Oct 21, 2020
@Gudahtt Gudahtt force-pushed the update-eslint-config branch from 1a15b85 to 4d20d00 Compare October 21, 2020 15:13
package.json Outdated
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this to use a ~ range because on v7.21.x I kept seeing false positives for the react/no-unused-prop-types, in cases where we use PropTypes.arrayOf(PropTypes.shape({ ... })) specifically.

I haven't found a similar bug report for this yet, so I'm going to do a little more digging and create one if necessary. In the meantime, I think ~7.20.0 has everything we need.

whymarrh
whymarrh previously approved these changes Oct 21, 2020
Copy link
Copy Markdown
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

`@metamask/eslint-config` has been updated to v4.0.0. This update
requires that we update `eslint` to v7 as well, which in turn requires
updating most `eslint`-related packages.

Most notably, `babel-eslint` was replaced with `@babel/eslint-parser`,
and `babel-eslint-plugin` was replaced by `@babel/eslint-plugin`. This
required renaming all the `babel/*` rules to `@babel/*`.

Most new or updated rules that resulted in lint errors have been
temporarily disabled. They will be fixed and re-enabled in subsequent
PRs.
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [ba69b31]
Page Load Metrics (335 ± 18 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint29393231
domContentLoaded2924183343718
load2944203353718
domInteractive2924183333718

Copy link
Copy Markdown
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Gudahtt Gudahtt merged commit 7d0a7ab into develop Oct 21, 2020
@Gudahtt Gudahtt deleted the update-eslint-config branch October 21, 2020 16:31
@github-actions github-actions bot locked and limited conversation to collaborators Oct 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants