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

Linting and Formatting #133

Merged
merged 5 commits into from
Jul 1, 2024
Merged

Linting and Formatting #133

merged 5 commits into from
Jul 1, 2024

Conversation

wilwade
Copy link
Member

@wilwade wilwade commented Jul 1, 2024

Review Note: I split this into different commits so it would be easy to review that which wasn't automatically applied.

I suggest reviewing those commits instead of every line.

Problem

Styles and potential issues were all over without enforcing linting and formatting.

Closes: #34

Solution

Added linting and formatting.

Change summary:

  • Added eslint
  • Added Prettier
  • Checked to make sure it was setup in CI

Steps to Verify:

  1. npm i
  2. npm run lint
  3. npm run format
  4. npm run lint:fix

@wilwade wilwade force-pushed the chore/lint-format-34 branch from 863e85b to 91ae9ac Compare July 1, 2024 14:00
Copy link
Collaborator

@mattheworris mattheworris left a comment

Choose a reason for hiding this comment

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

  • Scanned through commits, nothing jumped out at me
    🚢 it

Copy link
Collaborator

@shannonwells shannonwells left a comment

Choose a reason for hiding this comment

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

I scanned the changes starting w/ the last file & going up, FWIW, all looks like normal formatting/style lints.

There are at least two sets of rules in the default settings that IMO are inconsistent and/or make the code less readable and not more. We can quibble about that later, since the point of this is just to get the linting and formatting integrated.

let reader;

const listArraySchema = new parquet.ParquetSchema({
Copy link
Collaborator

Choose a reason for hiding this comment

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

just noting this was moved up

@wilwade wilwade merged commit 6396d26 into main Jul 1, 2024
1 check passed
@wilwade wilwade deleted the chore/lint-format-34 branch July 1, 2024 16:42
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.

Add Linter and Formatter
3 participants