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

Improve linter, formatter config #613

Closed
wants to merge 2 commits into from
Closed

Conversation

rotu
Copy link
Contributor

@rotu rotu commented Jun 13, 2024

  1. Streamline by switching eslint-config-atomic -> @typescript-eslint.
  2. Allow prettier config to be run separately and use .prettierrc to ignore yaml and md files.
  3. Bump prettier. Prettier 3 automatically ignores .gitignore'd files. Also prettier introduces a breaking change on nearly every release.

Note I did not migrate to eslint 9 with the new flat config setup, but it seems a reasonable next step.

rotu added 2 commits June 13, 2024 16:49
1. Streamline by switching eslint-config-atomic -> @typescript-eslint.
2. Allow prettier config to be run separately and use `.prettierrc` to ignore yaml and md files.
3. Bump prettier. Prettier 3 automatically ignores .gitignore'd files. Also prettier introduces a breaking change on nearly every release.
Comment on lines -1 to -6
{
"extends": [
"../.eslintrc",
"eslint-config-atomic/strict",
]
}
Copy link
Member

@aminya aminya Jun 14, 2024

Choose a reason for hiding this comment

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

Eslint-config-atomic already supports TypeScript and more underneath
https://github.com/atom-community/eslint-config-atomic/blob/master/src/typescript.cts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. It also pulls in a ton of other stuff (including conflicting peer dependencies)
  2. eslint-config-atomic is failing CI and I suspect it will have trouble with eslint 9.
  3. It's not clear which rules are in use and why. ZeroMQ is not part of the Atom project.

What particular rules from eslint-config-atomic am I missing that add value?

Copy link
Member

@aminya aminya left a comment

Choose a reason for hiding this comment

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

Could you separate the Prettier change into a separate PR? I'm not sure if I want to downgrade the linter config from eslint-config-atomic.

@rotu
Copy link
Contributor Author

rotu commented Jun 14, 2024

Could you separate the Prettier change into a separate PR? I'm not sure if I want to downgrade the linter config from eslint-config-atomic.

I'll be happy to, eventually. Right now, this PR really still belongs in draft status.

I'm confused by the entanglement between prettier and eslint.

  • This project previously had a direct dependency on prettier@^2.8.0 and indirect on prettier@^2.6.2 via eslint-config-atomic. Moving forward, how can we prevent these transitive dependencies from creeping in?

@rotu rotu marked this pull request as draft June 14, 2024 08:12
@aminya aminya closed this in #615 Jun 14, 2024
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.

2 participants