Skip to content

Removing childprocess for rmRF#1373

Merged
vmjoseph merged 39 commits into
mainfrom
users/vmjoseph/rmrf-windows
Mar 15, 2023
Merged

Removing childprocess for rmRF#1373
vmjoseph merged 39 commits into
mainfrom
users/vmjoseph/rmrf-windows

Conversation

@vmjoseph

@vmjoseph vmjoseph commented Mar 14, 2023

Copy link
Copy Markdown
Contributor
  • 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
Comment thread packages/io/__tests__/io.test.ts Outdated
Comment thread packages/io/__tests__/io.test.ts Outdated
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
Copy Markdown
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.

Comment thread packages/io/src/io-util.ts
Comment thread packages/io/src/io-util.ts
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>
Comment thread packages/io/__tests__/io.test.ts Outdated
Comment thread packages/io/__tests__/io.test.ts Outdated
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.

3 participants