rocktemplates / rock

Generate file structures or project skeletons from predefined templates.
MIT License
14 stars 1 forks source link

Allow delete array in .rock/rock.json #23

Closed RyanZim closed 8 years ago

RyanZim commented 8 years ago

Fixes #22.

I don't really like to use sync methods, but using async methods would require pulling in an async management lib.

jprichardson commented 8 years ago

but using async methods would require pulling in an async management lib.

Why would we need an async lib? Not that I'm opposed to one, just not sure that it's needed.

RyanZim commented 8 years ago

If we iterate over an array, calling fs.remove() on each item, we need to know when all of the remove() calls are finished, allowing us to call the main callback.

jprichardson commented 8 years ago

If we iterate over an array, calling fs.remove() on each item, we need to know when all of the remove() calls are finished, allowing us to call the main callback.

Of course. But we don't need an async lib for that. Yes, it makes it easier, especially if we want to execute concurrently as opposed to sequentially, but not required. Btw, I'm still fine with using an async library.

RyanZim commented 8 years ago

if we want to execute concurrently as opposed to sequentially

In a CLI context, executing async methods sequentially has no benefits over sync methods that I can think of.

What's your preference of async lib?

jprichardson commented 8 years ago

What's your preference of async lib?

Early on, I thought it was really important to have a lean and mean async lib because async.js is so heavyweight. So like all eager devs, I built my own. batchflow and queueflow were born. These days I don't use any async lib and just execute async sequentially a la (in most cases):

function asyncOpt () {
  if (someCondition) return
  // do something
  fs.remove(someFile, asyncOpt)
}

asyncOpt()

But like you said, for a CLI tool, sync is fine. However, if we want to allow people plug Rock into there own app programmatically, sync is not fine.

So we must ask ourselves, should we optimize our dev efforts to get Rock CLI working really well? IDK. Depends upon your objectives.

I could wax poetic about this stuff for awhile. But the older I get, at the end of the day, I find that it really just doesn't matter. And all that matters is working code and results.

So, to finally answer your question... I support which every library or absence of library that makes you the most productive, the most happy, and is in accordance with your own personal goals.

I state the aforementioned with all sincerity as well :)

jprichardson commented 8 years ago

One more thing that I thought of... if you haven't looked at Babel and async/await that may be worth your time. I personally enjoy using Babel with async/await as it makes async code very clean. Eventually Node.js will add async / await support, maybe Node v7 in Oct? And then we could drop Babel altogether eventually.

RyanZim commented 8 years ago

However, if we want to allow people plug Rock into there own app programmatically, sync is not fine.

I would guess 75% of the time, programmatic users are not going to use delete, which means this won't be used much anyhow. For now I'd just leave it as it is.

I'm not flatly opposed to async.js; https://github.com/feross/run-parallel and friends look pretty good too. For now, I don't think it's needed.

Not in favor of babel, preprocessing is sometimes a necessary evil, but I prefer keeping to node core, at least for now.