Skip to content

Update ESLint configuration and packages#118

Merged
Gudahtt merged 5 commits intomainfrom
update-eslint-configuration
Oct 28, 2022
Merged

Update ESLint configuration and packages#118
Gudahtt merged 5 commits intomainfrom
update-eslint-configuration

Conversation

@Gudahtt
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt commented Oct 28, 2022

The ESLint configuration and all related dependencies have been updated. Most changes are related to the JSDoc lint rules; many JSDoc blocks were added or adjusted. All other lint changes were applied automatically using yarn lint:fix.

The ESLint configuration and all related dependencies have been
updated. Most changes are related to the JSDoc lint rules; many JSDoc
blocks were added or adjusted. All other lint changes were applied
automatically using `yarn lint:fix`.
@Gudahtt Gudahtt marked this pull request as ready for review October 28, 2022 14:05
@Gudahtt Gudahtt requested a review from a team as a code owner October 28, 2022 14:05
Comment thread src/changelog.ts
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).`;

interface ReleaseMetadata {
type ReleaseMetadata = {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I've seen this change made in a few commits and I'm curious about the motivation behind the shift from interface to type. Is this something we are slowly changing across the project, or is there a specific need that we are addressing?

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.

See here for more context: MetaMask/eslint-config#216 (and this Slack thread for even more context)

Primarily it was for consistency. No specific need being addressed. Erik referenced some difficulties encountered working with interfaces and our Json type, but I can't recall the details on that.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is great, thanks for the context @Gudahtt!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Erik referenced some difficulties encountered working with interfaces and our Json type, but I can't recall the details on that.

It has to do with interfaces not having an index signature, IIRC.

Copy link
Copy Markdown
Member

@Mrtenz Mrtenz left a comment

Choose a reason for hiding this comment

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

Looks good overall. Just some nits.

Comment thread src/changelog.ts Outdated
Comment thread src/changelog.ts Outdated
Comment thread src/changelog.ts Outdated
Comment thread src/cli.ts Outdated
Comment thread src/cli.ts Outdated
Gudahtt and others added 3 commits October 28, 2022 15:26
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
@Gudahtt Gudahtt merged commit 53a6d42 into main Oct 28, 2022
@Gudahtt Gudahtt deleted the update-eslint-configuration branch October 28, 2022 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants