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

chore: workspace cleanups #5191

Merged
merged 5 commits into from
Feb 24, 2025
Merged

chore: workspace cleanups #5191

merged 5 commits into from
Feb 24, 2025

Conversation

arendjr
Copy link
Contributor

@arendjr arendjr commented Feb 24, 2025

Summary

As I'm preparing to implement a Workspace watcher, I noticed a few minor code cleanups I wanted to do. I initially hit a bit of a dead end with my watcher approach, but I think the cleanups still make sense to merge to prevent a large PR down the line. Specifically:

  • Moved the init_thread_pool() function, since it doesn't really seem to belong inside the scanner.
  • Changed scan() to be a method on WorkspaceServer instead of a standalone function. Not really important, but seeing how the first method was a reference to the WorkspaceServer anyway, this seems cleaner. (I was going to do the same for the watcher, but that didn't work out.)
  • Move the workspace tests from tests/workspace.rs to src/workspace.tests.rs. This has the advantage that we can move an inline test that is in src/workspace.rs there as well, and we only need to keep one snapshots folder instead of two. It also saves a lot of indentation in the file.
  • Move the version number for static_assertions to the top-level Cargo.toml.

Test Plan

CI should remain green.

@arendjr arendjr requested review from a team February 24, 2025 09:10
@github-actions github-actions bot added A-Core Area: core A-Project Area: project L-JSON Language: JSON and super languages labels Feb 24, 2025
Copy link
Member

@ematipico ematipico 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!

Copy link

codspeed-hq bot commented Feb 24, 2025

CodSpeed Performance Report

Merging #5191 will not alter performance

Comparing arendjr:workspace-cleanups (348afda) with main (e8b32d5)

Summary

✅ 97 untouched benchmarks

@arendjr arendjr merged commit 2cf8355 into biomejs:main Feb 24, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Core Area: core A-Project Area: project L-JSON Language: JSON and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants