Should glob-stream and globby be merged? about glob-stream HOT 14 CLOSED

Why? glob-stream already had all of these features and better perf. What's the point in putting a bunch of time to turn globby into an identical module? I'm not seeing it

from glob-stream.

It is because every improvement I find must be pushed to two different packages, and that seems like a waste of time. sindresorhus/globby#6 fixes the perf issues.

from glob-stream.

@UltCombo Who said you had to do that? I know I never did. I think you decided on your own to update globby to achieve parity with glob-stream

from glob-stream.

If @sindresorhus wants to deprecate his module in favor of this one that is up to him, but from my perspective this one has more features.

from glob-stream.

@contra more precisely, I've updated globby to fix perf issues only so far. I haven't decided to achieve parity (yet), that is why I've opened the issue in the first place.

It would be nice if we could at least abstract away some common logic so that I don't have to push updates to both packages. But no hard feelings if that doesn't work out.

from glob-stream.

@UltCombo Why are you applying the same changes to both though? I'm still not understanding your logic here. Which one do you use?

from glob-stream.

@contra I believe @sindresorhus will not deprecate globby as it has a very different API at the moment (async and sync instead of streaming).

from glob-stream.

Can you explain in detail what your thought process is concerning why the modules should be merged? Like really walk me through it, I'm not grasping it yet

from glob-stream.

(hah, spaghetti replies order)

Why are you applying the same changes to both though?

Because there are improvements that apply to both. E.g. the Minimatch class usage for negative globs (#30 and sindresorhus/globby#6). After thinking a bit more, a direct dependency relation most likely wouldn't work out, but such common logic could possibly be abstracted away to another module. (I haven't examined this throughout yet though)

Which one do you use?

Not sure what you mean. If you recall, I'm using gulp-src-ordered-globs, which is another package duplicating the very same logic applied to glob-stream and globby. I'm using gulp-src-ordered-globs because this logic hasn't landed on stable gulp yet, and gulp-src-ordered-globs wraps gulp.src() which uses the old glob-stream logic.

from glob-stream.

Oh I thought a comment was deleted but it was just out of order (different comment order when I opened this thread in another tab somehow).

Anyway, yes, a direct dependency on globby wouldn't work. I'd just like to abstract away the common pain points (extend node-glob to accept multiple ordered globs), so that the same code can be applied to glob-stream, globby and perhaps gulp-src-ordered-globs.

from glob-stream.

@contra

Can you explain in detail what your thought process is concerning why the modules should be merged? Like really walk me through it, I'm not grasping it yet

Here's my line of thought:

• I thought globby (a node-glob extension with multiple patterns support) was bare bones enough that it could be used here, since they share some of the same core functionality (extend node-glob to support multiple patterns).
• But for globby to be properly consumed here, it would need a streaming or events API.
• By adding a streaming API to globby, then both packages would be pretty much identical.

Can you see my point now? (I'm slightly sick, not sure if my thoughts are making sense :P)

Anyway, you helped me realize that all of this would be pretty much useless (and we already had reached this conclusion in this thread as well).

But it would be really nice to be able to abstract away the "node-glob multiple ordered patterns" feature so that it can be consumed by both packages. This is what I was trying to achieve since the beginning, but ended up overcomplicating it (see line of thought bullet list above).

Both packages create a glob per positive match and then filter out all posterior negative globs. Right now, this is done via the Minimatch class, but soon enough node-glob will implement an ignore API and then I (or someone else) will have to PR this change to both glob-stream and globby. See what I mean?

from glob-stream.

Yeah, I think that's fine though.

from glob-stream.

Yeah, it is not a huge thing, I just thought it'd be nice to have this common logic maintained in a single repository.

Though yeah, globby itself implements this logic in two different ways (for async and sync methods), and glob-stream consumes a complete differently API (node-glob's events API instead of the callback one), so an abstraction wouldn't work well.

from glob-stream.

If such an abstraction was made, it would most likely end up as another clone of glob-stream.

from glob-stream.