-
Notifications
You must be signed in to change notification settings - Fork 19
chore(ignore-failure): ignore copy failures #483
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #483 +/- ##
==========================================
+ Coverage 73.86% 73.93% +0.07%
==========================================
Files 109 108 -1
Lines 10356 10326 -30
Branches 686 685 -1
==========================================
- Hits 7649 7635 -14
+ Misses 2704 2688 -16
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
To be clear, I'm not trying to invalidate the work of the other PRs by reverting it, I'm only brainstorming an alternative solution based on the fact that we are still seeing failures post #480 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the asset copying mechanism by replacing the custom safeCopy utility function with Node.js's native fs.cp() method, wrapped in a try-finally block to handle potential parallel execution conflicts.
- Removes the custom
safeCopyutility function and its file - Replaces
safeCopywith Node.js's nativefs.cp()for copying assets - Adds error handling via try-finally block to gracefully handle parallel execution failures
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/generators/legacy-html/utils/safeCopy.mjs | Entire file removed as custom copy logic is no longer needed |
| src/generators/legacy-html/index.mjs | Replaced safeCopy with native fs.cp() and added try-finally block for parallel execution safety |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Change finally block to catch block to handle potential failures gracefully.
This is an alternative to #480 and #482, which just ignores all failures.
I feel that if #482 doesn't work, this is the next best option.
cc @ovflowd @flakey5 @aduh95