Skip to content
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

Update ESLint configuration and packages #118

Merged
merged 5 commits into from Oct 28, 2022
Merged

Conversation

Gudahtt
Copy link
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
@@ -13,7 +13,7 @@ const changelogDescription = `All notable changes to this project will be docume
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).`;

interface ReleaseMetadata {
type ReleaseMetadata = {

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
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.

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
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
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.

src/changelog.ts Outdated Show resolved Hide resolved
src/changelog.ts Outdated Show resolved Hide resolved
src/changelog.ts Outdated Show resolved Hide resolved
src/cli.ts Outdated Show resolved Hide resolved
src/cli.ts Outdated Show resolved Hide resolved
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.

None yet

3 participants