Node-fs-extra: remove unnecessary aliases

Created on 10 Aug 2015  路  15Comments  路  Source: jprichardson/node-fs-extra

Inspired by this: https://github.com/ramda/ramda/pull/574#issuecomment-64124850 most aliases should be removed...

  • [x] remove delete() alias to remove(). Most style guides frown upon using a reserved keyword for a field / method name. So this should definitely be removed.

Questions:

  1. Should ensureDir()/mkdirp()/mkdirs() be condensed into one?
  2. json methods... should it be Json or JSON? I'm inclined to keep both of these variations but keep promoting Json.

Most helpful comment

readJSON would look a lot better, because JSON is an uppercase acronym. mkdirp seems best

All 15 comments

Just my 2cnt:

  1. mkdirp is best know - i'd reduce it to that (remove the others)
  2. Technically JSON is an acronym and as such should be upper-case (the JSON object is upper-case as well)
  1. Definitely remove mkdirs, IDK about ensureDir.
  2. I'm inclined to keep both of these variations but keep promoting Json.

    :+1:

Definitely remove mkdirs

I thought mkdirs was more newbie-friendly than mkdirp. Interesting. Of the three, I prefer to have only one way. ensureDir seems very clear to me... but I think it would confuse people not having mkdirp. IDK here.

LOL, I guess part of what makes me prefer mkdirp is the fact that I write quite a few npm scripts, so I tend to think in terms of bash commands.

Perhaps push this off to post-v1.0.0?

@jprichardson Perhaps we can revisit this for v2? I really don't know what to think.

@manidlou Would like to know your opinions here.

I am more inclined to keep mkdirp mainly because I use bash a lot; therefore I am more comfortable with mkdirp. Between JSON and Json, I go with Json. IDK, having a function name with all uppercase as the second word like readJSON looks a little peculiar to me! But it's just me! I am still open to both options though.

readJSON would look a lot better, because JSON is an uppercase acronym. mkdirp seems best

Personally, I'd prefer readJSON because, as said before, JSON is the proper capitalization, but use readJsonSync when I need to integrate with mandatorily synchronous code, because I find readJSONSync looks kind of weird. Generally, I'd go with JSON.

As for mkdirp et al., I'm personally not so fond of making everything bashy. However, seeing as the base fsis setup like that anyway, I'd stick with it. Therefore, mkdirp seems most appropriate for a package that seeks to be a seemless drop-in replacement.

@rasenplanscher I would still go for readJSONSync, because I would assume that you should just be able to add a Sync onto the end without worrying about changing capitalisation.

@jprichardson @manidlou @JPeer264 given the amount of divergence in ideas here, and how much code in the wild would be affected, I vote we close this and keep the aliases as they stand for now, but make an effort not to add new aliases in the future.

I read through the comments here, it seems that everybody has different prefs. As this library has got those aliases over some time now, it might be a bit confusing if the one or other preferred alias is not available anymore in the next major release. I would also suggest to keep those aliases, but don't add any new aliases in the future.

I would just remove aliases if they don't make sense, but here, all aliases make sense for me.

Agreed!

Closing for now as per https://github.com/jprichardson/node-fs-extra/issues/171#issuecomment-363184387; @jprichardson reopen if you disagree.

Was this page helpful?
0 / 5 - 0 ratings