Navigation Menu

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

es6 refactoring #355

Closed
JPeer264 opened this issue Feb 13, 2017 · 4 comments · Fixed by #372
Closed

es6 refactoring #355

JPeer264 opened this issue Feb 13, 2017 · 4 comments · Fixed by #372

Comments

@JPeer264
Copy link
Collaborator

Since just Node v4+ is supported wouldn't it be could to rewrite everything into ES6?

@RyanZim
Copy link
Collaborator

RyanZim commented Feb 14, 2017

Yeah, Node 4+ supports most, but not all ES6 features. Given that this is a large codebase, porting to ES6 will take some time.

Currently, I've had a fairly passive approach to this; when I worked on a certain part of the codebase, I would port it to ES6.

If you want to work on this, thought, PRs welcome.


@jprichardson Do you have ES6 stylistic preferences? (i.e. should var be used)

@jprichardson
Copy link
Owner

@jprichardson Do you have ES6 stylistic preferences? (i.e. should var be used)

We should avoid var and use let/const if we can.

This was referenced Feb 14, 2017
@manidlou
Copy link
Collaborator

@jprichardson @RyanZim regarding the codebase, both internally and for the exported functions, do we want to keep the callback style or we want to change that to promise-based?

@RyanZim
Copy link
Collaborator

RyanZim commented Feb 21, 2017

@manidlou: @jprichardson & I have discussed promise support via email in the past. It's something we definitely want to do sometime, but we want to do it right. That means:

  • We need to support both promises & callbacks; if a callback is passed, use it; otherwise return a promise.
  • We need to wrap the native fs methods as well.

We also want a solution that will allow incremental porting of the internals to promises (i.e. our solution must be able to wrap both promise and callback functions).

I've been planning on attacking this as soon as I get time; I wanted to do it for v2, but it didn't happen.

This was referenced Feb 22, 2017
JPeer264 added a commit to JPeer264/node-fs-extra that referenced this issue Feb 25, 2017
bluelovers added a commit to bluelovers/node-fs-extra that referenced this issue Mar 7, 2017
* master: (59 commits)
  Document `outputFile()` only supports a file path
  move(): fall back to streams when hard links aren't supported
  Refactor test.js
  Refactor test remove (closes jprichardson#355)
  Refactor test move
  Refactor test mkdirs
  lib/copy/__tests__/copy-dev-null.test.js: Edit copy-dev-null test description
  docs/copy.md: Refactor copy filter example
  Add done() to copySync tests when setTimeout() is used
  Remove unnecessary param from filter regex in copy and copySync, remove done() from copySync test
  Update: arrow functions simplified
  Refactor docs
  Refactor test output
  Refactor test util
  Refactor test json
  Refactor test ensure
  Add dest parameter to filter option in copy and copySync
  Refactor test empty
  Refactor missing var's
  Refactor test copy-sync
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants