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

replace unlink with rm #165

Merged
merged 1 commit into from
Dec 4, 2024
Merged

Conversation

felabr
Copy link
Contributor

@felabr felabr commented Jul 16, 2024

No description provided.

@felabr felabr force-pushed the replace-unlink-with-rm branch from 0cd60f1 to 3c6ae8b Compare July 16, 2024 17:20
@felabr
Copy link
Contributor Author

felabr commented Jul 17, 2024

sorry, that should have gone to my fork, and not to your repo

@felabr felabr closed this Jul 17, 2024
@nonara
Copy link
Owner

nonara commented Jul 18, 2024

Np. @felabr If I may ask, why did you need to swap it?

@felabr
Copy link
Contributor Author

felabr commented Jul 19, 2024

Sure, there's some information in this discussion: #164

Our GitHub pipelines sometimes fail when running ts-patch, and the error message tells me unlinkSync cannot find the lock file. Yes, I've seen the existsSync. Anyway, rmSync has this nice force option that would make it ignore missing files.

@nonara nonara reopened this Jul 23, 2024
@nonara
Copy link
Owner

nonara commented Jul 23, 2024

Thanks for sharing! Seems reasonable to consider this.

I'll need to check support on node versions and make sure there are no potential downsides. Will reopen for now

@felabr felabr force-pushed the replace-unlink-with-rm branch from 87e3a74 to 3c6ae8b Compare August 6, 2024 12:59
@felabr
Copy link
Contributor Author

felabr commented Aug 6, 2024

I cleaned it up a bit, the PR now only contains the unlinkSync / rmSync change.

@nonara nonara merged commit 3f6892f into nonara:master Dec 4, 2024
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