Skip to content

integrate eslint #81

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

Merged

Conversation

odama626
Copy link
Collaborator

@odama626 odama626 commented Aug 2, 2022

fixes #34

this adds eslint and moves to pretty-quick for prettier, but doesn't enable eslint yet -- want to get some of @tonyketcham's big Prs in first

@odama626 odama626 requested a review from tonyketcham August 2, 2022 18:04
@tonyketcham
Copy link
Collaborator

tonyketcham commented Aug 2, 2022

Looking great!

Before this goes in, we should:

  • start a branch off of this and subsequent draft PR that runs the formatting
  • in that new branch, we can fix all the places ESLint screams about quick and dirty stuff we've done (particularly around types 😛)
    • this will likely take some time and might need both of us to chip away at it to resolve all the errors

Then after this goes in, we can:

  • merge the ESLint formatting & fixes PR to main
  • merge this branch into any existing or stale branches to run the formatting prior to merging main into said branches.
    • this will avoid any merge conflicts from formatting changes

@odama626
Copy link
Collaborator Author

odama626 commented Aug 4, 2022

#89

only 4 errors remain, and its around types

  29:33  error  Don't use `object` as a type. The `object` type is currently hard to use ([see this issue](https://github.com/microsoft/TypeScript/issues/21732)).
Consider using `Record<string, unknown>` instead, as it allows you to more easily inspect and use the keys  @typescript-eslint/ban-types
  30:26  error  Don't use `object` as a type. The `object` type is currently hard to use ([see this issue](https://github.com/microsoft/TypeScript/issues/21732)).
Consider using `Record<string, unknown>` instead, as it allows you to more easily inspect and use the keys  @typescript-eslint/ban-types
  32:25  error  Don't use `object` as a type. The `object` type is currently hard to use ([see this issue](https://github.com/microsoft/TypeScript/issues/21732)).
Consider using `Record<string, unknown>` instead, as it allows you to more easily inspect and use the keys  @typescript-eslint/ban-types

 97 problems (4 errors, 93 warnings)```

@tonyketcham
Copy link
Collaborator

Nice one! Excited for this since it'll keep us in line & up our code standards

@odama626
Copy link
Collaborator Author

odama626 commented Aug 6, 2022

can we get this in? it's non-breaking, just really looking forward having pretty-quick

@odama626
Copy link
Collaborator Author

odama626 commented Aug 6, 2022

it's non breaking because lint:eslint isn't used by lint currently

@tonyketcham
Copy link
Collaborator

Yup - sorry for the delay on approving this one!

@odama626 odama626 merged commit b86d9c0 into FlatbreadLabs:main Aug 16, 2022
@odama626 odama626 deleted the all-the-eslint-none-of-the-fixes branch August 16, 2022 03:38
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.

Integrate ESLint
2 participants