miickel / gulp-angular-templatecache

Concatenates and registers AngularJS templates in the $templateCache.
MIT License
525 stars 103 forks source link

Replaced uses of event-stream with direct dependencies #168

Closed Basa0 closed 5 years ago

Basa0 commented 5 years ago

es.map and es.pipeline were just direct uses of map-stream and stream-combiner

Basa0 commented 5 years ago

Related to #166

I've made a similar pull request to gulp-footer https://github.com/tracker1/gulp-footer/pull/13

david-ellinger commented 5 years ago

Is there a timeline from when a PR is merged to when a version is released through NPM?

simonua commented 5 years ago

@david-ellinger, recently, turn-around has been relatively quick. Given the mess that is event-stream, I hope to look into this shortly. What we have had challenges with recently has been testing (gulp 3/4 & OS). It would be great if we could have feedback on these PRs for specific configurations. Side note: I realize there is no CONTRIBUTING.md in this repo, so I'll look to clarify things there a bit, too.

simonua commented 5 years ago

@Basa0, what version npm do you use? I am getting a different lock file after running npm i. I use latest/6.4.1.

Basa0 commented 5 years ago

@Basa0, what version npm do you use? I am getting a different lock file after running npm i. I use latest/6.4.1.

Same, 6.4.1. Just retried, and I get the same result.

simonua commented 5 years ago

Interesting. Ok, let's stay with the current lock file then.

simonua commented 5 years ago

Verified that only gulp-footer's reference to event-stream is left.

image

simonua commented 5 years ago

This PR seems like the right way to go. angular-template-cache uses event-stream 3.3.4. I looked at 3.3.4's index.js and verified these two dependencies were just pass-throughs, so this appears to add up.

As expected, there's no reference to flatmap-stream anywhere.

20 tests pass.

@adamreisnz & @dmellstrom, would you mind giving this a once-over real quickly, too, please? Yes, I am trying to rope you in a bit for community consensus, if that's ok. And, thank you!

adamreisnz commented 5 years ago

Looks good to me 👍

simonua commented 5 years ago

Thank you, sir!

solidevolution commented 5 years ago

Looks more secure than before ;) Thank you!

simonua commented 5 years ago

Hi @Basa0, if you could update gulp-footer to 2.0.2, then I think we're about done with this PR. Thanks for all your time and help here!