Skip to content

Replace incremental data sinks with a single stream of TaskEvent enums#23

Merged
jspahrsummers merged 7 commits intomasterfrom
combine-incremental-and-aggregate-results
Jun 1, 2015
Merged

Replace incremental data sinks with a single stream of TaskEvent enums#23
jspahrsummers merged 7 commits intomasterfrom
combine-incremental-and-aggregate-results

Conversation

@jspahrsummers
Copy link
Copy Markdown
Member

Depends on #22.

This is a refactoring that should've been done when we first moved from ColdSignal + HotSignal to SignalProducer + Signal.

The current sink-focused implementation bifurcates logic, and makes it really easy to introduce issues like that fixed by #21. Handling everything within the same stream should make behavior more deterministic.

@jspahrsummers jspahrsummers changed the title [WIP] Replace incremental data sinks with a single stream of TaskEvent enums Replace incremental data sinks with a single stream of TaskEvent enums May 28, 2015
@robrix
Copy link
Copy Markdown

robrix commented May 28, 2015

#22 has been merged.

@jspahrsummers
Copy link
Copy Markdown
Member Author

🙌

Comment thread ReactiveTask/Task.swift
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When terminationStatus != EXIT_SUCCESS, signal from stderrProducer does not wait signal from stdoutProducer. Signal from other hand does not be needed, but should be waited.
I guess this cause segmentation fault of Carthage/Carthage#497

changed comment position

@jspahrsummers
Copy link
Copy Markdown
Member Author

@norio-nomura Good catch yet again! I've pushed a fix for that issue.

@norio-nomura
Copy link
Copy Markdown
Contributor

@jspahrsummers
Copy link
Copy Markdown
Member Author

There are still issues here, but I think it's in our interest to make a less broken Carthage release available. I'd like to merge this, and continue to investigate the remaining issues separately.

Also, any help on that front would be greatly appreciated. 🙏

jspahrsummers added a commit that referenced this pull request Jun 1, 2015
…e-results

Replace incremental data sinks with a single stream of TaskEvent enums
@jspahrsummers jspahrsummers merged commit 3fedab1 into master Jun 1, 2015
@jspahrsummers jspahrsummers deleted the combine-incremental-and-aggregate-results branch June 1, 2015 03:22
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.

3 participants