| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
lgtm
Sorry, something went wrong.
There was a problem hiding this comment.
Just making sure this doesn't land without documentation for the Promises and sync APIs
Sorry, something went wrong.
There was a problem hiding this comment.
Looks good overall! 🙌
Sorry, something went wrong.
Sorry, something went wrong.
While we're rethinking options, should errorOnExist default to false or true? This is another one of those options that has its value for compat reasons. |
Sorry, something went wrong.
|
Also, for reference; discussion thread on how fs-extra should handle the forthcoming naming conflict: jprichardson/node-fs-extra#912 |
Sorry, something went wrong.
There was a problem hiding this comment.
Further simplifications on the subject of replacing open + futimes + close with utimes.
Sorry, something went wrong.
I was tempted to go with false, which I believe would make copying to the same folder twice relatively safe, i.e., you don't clobber files, but it's less of a pain in the neck to recover from a partial copy (you don't have to lookup an option). Thoughts? |
Sorry, something went wrong.
|
FYI, waiting on a windows machine at work (there are some hiccups with provisioning). Would happily pair with someone on fixing up the failing windows test, if anyone has a system up and running. |
Sorry, something went wrong.
Introduces recursive cp method, based on fs-extra implementation. PR-URL: #39372 Fixes: #35880 Refs: nodejs/tooling#98 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ian Sutherland <ian@iansutherland.ca>
Introduces recursive cp method, based on fs-extra implementation. PR-URL: #39372 Fixes: #35880 Refs: nodejs/tooling#98 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ian Sutherland <ian@iansutherland.ca>
|
Is there any place to give feedback on this function? Just looking through the docs, I think the option force: <boolean> is a bit ambiguous and would have been better suited to be named overwrite- as that's literally what it achieves, according to the docs. [EDIT] |
Sorry, something went wrong.
|
@lostpebble please feel free to open an issue. As much as possible I'd like to model after cp's flags, but perhaps clobber is the better name in this case? One good thing about force is it's consistent with rm. |
Sorry, something went wrong.
As per nodejs/node#39372 (comment) Resolves #918
| Back | FazBrowse Home | New Git URL |
Introduces recursive cp method, based on fs-extra implementation
Refs: nodejs/tooling#98
Fixes: #35880
Opening to start conversation.
TODO:
CC: @nodejs/tooling, @jprichardson, @manidlou, @RyanZim