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

Removing childprocess for rmRF #1373

Merged
merged 39 commits into from Mar 15, 2023
Merged

Removing childprocess for rmRF #1373

merged 39 commits into from Mar 15, 2023

Conversation

vmjoseph
Copy link
Contributor

@vmjoseph vmjoseph commented Mar 14, 2023

  • Uses node's rm instead of custom rmRF logic
  • Fixes locked file tests
  • Removes symlink tests for windows specific cases

@vmjoseph vmjoseph force-pushed the users/vmjoseph/rmrf-windows branch from 67155c8 to a730b5c Compare March 14, 2023 18:25
@vmjoseph vmjoseph changed the title Users/vmjoseph/rmrf windows Removing childprocess for rmRF Mar 15, 2023
@vmjoseph vmjoseph marked this pull request as ready for review March 15, 2023 14:18
@vmjoseph vmjoseph requested a review from a team as a code owner March 15, 2023 14:18
packages/io/__tests__/io.test.ts Outdated Show resolved Hide resolved
await assertNotExists(testPath)
// for windows, we need to explicitly set an exlcusive lock
// or we won't get an EBUSY error. this can be fixed after
// https://github.com/libuv/libuv/issues/3267 is resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this inside the try statement? It's about the EPERM comment. I don't think Node considers its fs.open call not creating a lock on Windows a bug.

packages/io/src/io-util.ts Show resolved Hide resolved
packages/io/src/io-util.ts Show resolved Hide resolved
vmjoseph and others added 2 commits March 15, 2023 10:53
Co-authored-by: Cory Miller <13227161+cory-miller@users.noreply.github.com>
Co-authored-by: Cory Miller <13227161+cory-miller@users.noreply.github.com>
vmjoseph and others added 3 commits March 15, 2023 13:53
Co-authored-by: Cory Miller <13227161+cory-miller@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants