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

please make sure the first argument in callback is error object #233

Closed
dead-horse opened this issue Apr 6, 2016 · 6 comments
Closed

please make sure the first argument in callback is error object #233

dead-horse opened this issue Apr 6, 2016 · 6 comments
Milestone

Comments

@dead-horse
Copy link

https://github.com/jprichardson/node-fs-extra/blob/master/lib/copy/ncp.js#L237

@jprichardson
Copy link
Owner

You're right, this should be fixed and be Error? (Error or null/undefined).

@jprichardson jprichardson added this to the 1.0 milestone Apr 6, 2016
@RyanZim
Copy link
Collaborator

RyanZim commented Oct 26, 2016

@jprichardson Can you elaborate a bit?

@jprichardson
Copy link
Owner

@RyanZim copy() sometimes returns an array of errors instead of an Error object. This should be fixed for 1.0.0. The problem is... what should the behavior be like? First error? First error with an array of the other errors?

@RyanZim
Copy link
Collaborator

RyanZim commented Oct 26, 2016

@jprichardson Let's face it: ncp.js need rewritten. The resulting rewrite would probably make it small enough to inline in https://github.com/jprichardson/node-fs-extra/blob/master/lib/copy/copy.js. I initially thought that perhaps we could keep it running long enough to survive the v1.0.0 release, but I'm not sure.

First error?

👍

Should all the issues pertaining to ncp.js be consolidated into one issue tracking the rewrite?

@jprichardson
Copy link
Owner

Should all the issues pertaining to ncp.js be consolidated into one issue tracking the rewrite?

I'm fine with this. It's changed significantly since the original ncp and has many bugs fixed. My major concern with the rewrite is that it delays the release of 1.0.0. May be best to do it for 2.0.0. But if you feel differently, than I'm more than fine with it for 1.0.0.

@RyanZim
Copy link
Collaborator

RyanZim commented Oct 26, 2016

@jprichardson Not set on it, but I'm afraid fixing all the bugs in ncp is going to take longer than rewriting it. 😀

RyanZim added a commit that referenced this issue Oct 26, 2016
- Fix #233 - First argument passed to callback should be an Error
  object, not an array.
- Fix #272 - Should error when options.clobber is false and the
  destination exists.
- Remove options.limit, it is no longer needed with graceful-fs.
RyanZim added a commit that referenced this issue Oct 26, 2016
- Fix #233 - First argument passed to callback should be an Error
  object, not an array.
- Fix #272 - Should error when options.clobber is false and the
  destination exists.
- Remove options.limit, it is no longer needed with graceful-fs.
RyanZim added a commit that referenced this issue Oct 27, 2016
- Fix #233 - First argument passed to callback should be an Error
  object, not an array.
- Fix #272 - Should error when options.clobber is false and the
  destination exists.
- Remove options.limit, it is no longer needed with graceful-fs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants