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

Add an NPM workspace and use prettier for non-rust source files #852

Merged
merged 7 commits into from
Feb 19, 2022

Conversation

kjvalencik
Copy link
Member

@kjvalencik kjvalencik commented Feb 11, 2022

Tip for reviewing: Review the individual commits instead of the PR. Ignore the "Prettier!" commit where files are formatted in mass; there are no code changes in that commit.

This is a very large diff because of prettier and lock files, but the actual changes are all in config files.

  • Make the root of the project an NPM workspace, creating a root package-lock.json
  • Move all NPM packages in the project to the workspace; this deletes their individual lockfile
  • Moved NPM tests from src/lib.rs executors into the workspace tests
  • Updated CI to call npm test instead of cargo test
  • Bumped package versions to unify the workspace and eliminate audit failures
  • Disabled lib checks in tsc to fix compilation; we can revert this after spectron is removed
  • Ran prettier on the entire project for all js, ts, json, and yml files (excluding generated)
  • Added a prettier check to our CI lint job

@dherman I used the default prettier configuration. If there's something about the formatting you don't like, let me know and we can tweak it. I prefer tabs, but we were already using two-spaces.

As a nice benefit, it also fixes the legacy backend on newer NPM versions.

@kjvalencik kjvalencik requested a review from dherman February 11, 2022 15:37
@kjvalencik kjvalencik force-pushed the kv/workspaces-and-prettier branch 29 times, most recently from b00b004 to 53944f1 Compare February 13, 2022 23:50
@kjvalencik kjvalencik force-pushed the kv/workspaces-and-prettier branch 2 times, most recently from ef1c88c to b846611 Compare February 14, 2022 13:50
Copy link
Collaborator

@dherman dherman left a comment

Choose a reason for hiding this comment

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

Lots of great cleanup! I'm excited to use workspaces and get rid of the recursive cargo test.

One suggestion in the comments, and then I'd suggest maybe just adding some brief docs to the main README on how to run the Neon tests (npm test in general, npm test --workspace=test/foo for individual subtests).

@kjvalencik kjvalencik force-pushed the kv/workspaces-and-prettier branch from 43d9684 to 5219fc5 Compare February 19, 2022 14:14
@kjvalencik kjvalencik merged commit 1c3d6f2 into main Feb 19, 2022
@kjvalencik kjvalencik deleted the kv/workspaces-and-prettier branch February 19, 2022 14:35
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