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

Merge with fs.extra and mkdirp #2

Closed
coolaj86 opened this issue May 22, 2012 · 14 comments
Closed

Merge with fs.extra and mkdirp #2

coolaj86 opened this issue May 22, 2012 · 14 comments

Comments

@coolaj86
Copy link

Would be nicer to have a single fs-extra or fs.extra module with all of the little extras in one.

https://github.com/coolaj86/node-examples-js/tree/master/fs.extra

https://github.com/substack/node-mkdirp

@jprichardson
Copy link
Owner

Hmm, good point. Your fs.extra contains a rename method that would be useful. And I use mkdirp a lot... so you're right, this should be done.

@coolaj86
Copy link
Author

Shall we move it all into your repo? my fs.copy also changes utimes, but yours uses .pipe. I think we should just add utime stamping to yours and keep both copyFile and copy an aliases for backwards compat.

I think copy and copyRecursive make for a better api than copyFile and copyDir.

What do you think?

@jprichardson
Copy link
Owner

Hehehe... looks like you just updated yours. ;p

@jprichardson
Copy link
Owner

My concern is one of API naming consistency. Node's own fs API naming scheme is inconsistent with itself. It's not awful in the PHP str_ library sense, but it could be improved.

fs.readdir() vs fs.readFile() I don't like like how 'F' in 'file' is capitalized and the 'd' in 'dir' is not capitalized. Maybe I'm being a bit pedantic.

Anyway, to what we are discussing, I don't mind either naming scheme as long as its consistent. As it stands now in your fs.extra you have copyRecursive() and mkdirp(). One name is using the unix scheme and other isn't.

The problem with this is that it's unpredictable and unintuitive to new users of the API. So, I'm in favor of naming them all similar. If you want to keep copyRecursive() I'm fine with that, but then I'd like new names for mkdirp and rmrf. Or vice versa.

Thoughts?

@coolaj86
Copy link
Author

@coolaj86
Copy link
Author

And either I need to create a new repo for the fs.extra stuff I have, or I can move my stuff to your repo.

Also, I found another library that's pulling in stuff
flatiron/utile#11

@jprichardson
Copy link
Owner

OK, +1 for consistency.

  1. What do you think about Node's fs API? It leans more towards terse names than verbose names. See: fs.rmdir() instead of naming it fs.removeDirectory()? Also fs.mkdir().
  2. Any major differences between your node-walk and https://github.com/nshah/nodejs-walker?

@jprichardson
Copy link
Owner

OK, I've decide that I'm going to implement all of these methods. But, I'm naming them in the terse unixy manner. The reason that I chose this is to be consistent with the naming scheme chosen by Node. Take a look at http://nodejs.org/api/fs.html#fs_class_fs_stats. With the exception of rename(), a lot of the methods are named the same as the unix command line programs. See: mkdir(), rmdir(), chown(), chmod().

@coolaj86 I'd still love to know any of your thoughts on this matter.

@coolaj86
Copy link
Author

Yes, I think the terse names are a great idea.

Yes, there are significant benefits to both your walk and mine.

My walk doesn't continue walking until you call next. That's important when you're going through a directory and taking the md5sum of all the files... and you have 30,000 files. I also emit nodeError (any inode) and directoryError separately because the project I was working on when I built it needed that distinction. I'm also using exactly the same name for the events as the node api.

Your method for filterDir, which is a lot simpler than mine and I like the chaining - makes the docs look much cleaner.
I feel that the next is a must for the walk library and it's fairly complicated to implement.

utile and wrench may both be better places to include these methods, as they already have some traction. utile will not accept CoffeeScript, unfortunately.

flatiron/utile#11

ryanmcgrath/wrench-js#29

https://github.com/substack/node-mkdirp/issues/17#issuecomment-5864594

@jprichardson
Copy link
Owner

(To be clear, the walker that I presented as a comparison to yours, isn't mine.)

Wow, I just read through all of those including @isaacs great essay. It's almost as if everyone read your words, but no one quite understood what you said, so to speak. I have massive respect for these individuals, but it almost seems as if everything went right over their head.

Here are your points, as I understand them:

  1. You've noticed that there is a lot duplication of work regarding the grouping of filesystem functions.
  2. Because of this duplication, there are a lot of modules that people have to choose between.
  3. You're looking to build an alliance in an effort to bless and work on one module of filesystem functions outside of Node core.
  4. You're not stating that all of this functionality should be actually written and included in this extra module. But making it available is what you're after: linking @substack mkdirp in the module and making it available is acceptable and encouraged.
  5. You don't care that it's written in JavaScript, CoffeeScript or tested in Mocha, Tap, etc.

If I've accurately stated your points, then I'm not sure how the discussion in https://github.com/substack/node-mkdirp/issues/17#issuecomment-5874346 missed your points.

Did I accurately state your points? By the way, as a fellow member of the Node community, I appreciate your pragmatic thinking on this matter.

@coolaj86
Copy link
Author

Yes, those are my points.

But people didn't see the ideas as distinctly as you. The conversation morphed from one topic to another and it seems that some people didn't follow that the topic was changing and thought that upcoming arguments were in support of the original topic (let's get the author to give up mkdirp in favor of blessed fs tools).

The other thing is that people sway from top to bottom. i.e. manic depression. If you were just in an iron-fisted community your next community will naturally be one of anarchy. Over time you decide you don't like anarchy and switch back to something almost iron-fisted, then you swing back to something not quite anarchy, and eventually you land in the middle.

I think it's a herd mentality of people who were slapped in the face by either side of the argument and still believe that the middle is too close to the side that bit them. Give it a few years and node will be closer to the middle and a new community will surface that's either iron-fisted or anarchic and the pattern with repeat.

@thlorenz
Copy link

thlorenz commented Feb 5, 2013

I know this discussion is old, but IMO this module should be split up into several modules, one for each action, i.e. copy, create, etc.,

That would alleviate the need to merge with mkdirp since then I can just pull in whatever I need i.e. if I need to copy a file I'll npm i copy-file and if I need to create a directory I can choose between mkdirp or any other module that does the job.

@cadorn
Copy link

cadorn commented Feb 8, 2013

-1 for splitting package. This package is great because it combines several others into one API. If you just need some of it use the underlying modules.

@jprichardson
Copy link
Owner

This is old enough that it can be closed. It spawned excellent discussion and encouraged a lot of thinking on my end. Directions have been established and momentum has been achieved. I'm still open and willing to collaborate or perhaps even join forces, but if that should happen, let's move that over to a new discussion.

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

No branches or pull requests

4 participants