Skip to content

reconstruct outputs every addHooks (to make it compatible with watchify) #71

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

jcppman
Copy link

@jcppman jcppman commented Jun 19, 2015

fix #68, #63

  • To avoid writing to ended writeStream, outputs will be constructed everytime
  • In the scenario that user wanna use outpipe, the outopt will be
    kept as string, so we can determine the user's attempt in later codes
    and do the relavant opperations

@jcppman jcppman changed the title reconstruct outputs everytime reconstruct outputs every addHooks Jun 19, 2015
@jcppman jcppman changed the title reconstruct outputs every addHooks reconstruct outputs every addHooks (to make it compatible with watchify) Jul 25, 2015
@dgbeck
Copy link
Contributor

dgbeck commented Jul 26, 2015

Thanks @jcppman for this patch. However this guy does not work for the case where outputs is an array of streams. I think we may need something like this to fix the issue

https://github.com/Aben/factor-bundle/commit/18692d69d983470857d6d3c6af7b7bd12266714f

jcppman added 3 commits July 26, 2015 18:44
- To avoid writing to ended writeStream, outputs will be constructed everytime
- In the scenario that user wanna use outpipe, the `outopt` will be
kept as string, so we can determine the user's attempt in later codes
and do the relavant opperations
sorry, the previous location is better
@jcppman
Copy link
Author

jcppman commented Jul 26, 2015

added two tests which will fail before applying the patch and will success after applying the patch

dgbeck added a commit to dgbeck/factor-bundle that referenced this pull request Jul 29, 2015
@dgbeck
Copy link
Contributor

dgbeck commented Jul 29, 2015

Just opened #75 that incorporates these changes and tests, and also added ability for outputs to be specified as a function for the stream case. Thanks @jcppman.

@jcppman
Copy link
Author

jcppman commented Jul 29, 2015

@dgbeck

In my opinion https://github.com/Aben/factor-bundle/commit/18692d69d983470857d6d3c6af7b7bd12266714f is more of a workaround to skip the part that cause the issue (write to end stream).

BTW how about three-ways merging instead of copy-and-paste? I mean, I'm not quite care if i got my name on the commit or not but i guess that's the way open-sourced things work right?

@dgbeck
Copy link
Contributor

dgbeck commented Jul 29, 2015

Sorry u are right. did not mean to side step credit. will do the merge in the future. Thx for vocalizing.

Just want to make sure the stream case is covered as we need it. If there is a better way then let's do it that way

@jcppman
Copy link
Author

jcppman commented Jul 29, 2015

@dgbeck No worry, I'm glad that it's useful to other ppl not just me!

@ghost
Copy link

ghost commented Jul 30, 2015

all sorted in the 2.5.0 release

@ghost ghost closed this Jul 30, 2015
@jcppman jcppman deleted the newOutputs branch July 30, 2015 05:29
@jcppman
Copy link
Author

jcppman commented Jul 30, 2015

Fantastic!

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken command line outpipe
2 participants