Optionally use user-provided output streams#65
Open
eggyal wants to merge 13 commits intoatomify:masterfrom
eggyal:providedOutputStreams
Open
Optionally use user-provided output streams#65eggyal wants to merge 13 commits intoatomify:masterfrom eggyal:providedOutputStreams
eggyal wants to merge 13 commits intoatomify:masterfrom
eggyal:providedOutputStreams
Conversation
Contributor
|
@eggyal cool! |
Contributor
|
Can you rebase on master (so this can be merged) and add some tests (and some docs)? |
Author
|
@joeybaker: Rebase, tests and docs all done. |
Author
|
Hm. There is actually a small problem when using this together with #63, since then the same streams are provided on each Perhaps instead of passing in an array of streams we should pass in a stream generator function that atomify-js would then call on each rebundle? What do you think, @joeybaker? |
prevents intermittent failures.
The documentation for [`_.values(object)`](https://lodash.com/docs#values) gives an example: > _.values(new Foo); > // → [1, 2] (iteration order is not guaranteed) Since "iteration order is not guaranteed", the array of streams passed to factor-bundle may not be in the same order as the corresponding files in `entries`. Factor bundle [depends on this correlation](https://github.com/substack/factor-bundle/blob/master/index.js#L73) when building its pipeline. This commit ensures that there is never a mismatch.
The [documentation for `stream-buffers` states](https://github.com/samcday/node-stream-buffer#disclaimer): > # Disclaimer # > > Not supposed to be a speed demon, it's more for tests/debugging or weird edge cases. It works with an internal buffer that it copies contents to/from/around. Moreover, users may wish to stream the results anyway, so buffering and then converting to a string only then to stream therefrom is rather wasteful.
Author
|
Now using a generator function as described above. |
Contributor
|
@eggyal nifty! The generator function makes a ton sense. My only nit-pick at this point, is that it looks like the rebase pulled in some commits to master. I can fix that up for you if you prefer? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The documentation for
stream-buffersstates:Moreover, users may wish to stream the results anyway, so buffering and then converting to a string only then to stream therefrom is rather wasteful.