Skip to content
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

Ignore Sync errors #347

Merged
merged 5 commits into from
Mar 14, 2017
Merged

Ignore Sync errors #347

merged 5 commits into from
Mar 14, 2017

Conversation

Ulexus
Copy link
Contributor

@Ulexus Ulexus commented Mar 6, 2017

Take the simplest approach to fixing the cross-platform compatibility issues with syncing stderr and stdout (outlined in #328) and just ignore sync errors. If we come up with a clean solution to #370, we can revert this change.

Fixes #328.

@mention-bot
Copy link

@Ulexus, thanks for your PR! By analyzing the history of the files in this pull request, we identified @akshayjshah to be a potential reviewer.

@CLAassistant
Copy link

CLAassistant commented Mar 6, 2017

CLA assistant check
All committers have signed the CLA.

@bufdev
Copy link
Contributor

bufdev commented Mar 6, 2017

I'm not sure just ignoring a sync error is the best way to go about this - any other ideas?

@akshayjshah
Copy link
Contributor

I'm going to push some commits to take this PR in a different (simpler) direction. However, we really appreciate the time you've taken on this - I'll make sure to preserve your authorship of the changes.

@Ulexus
Copy link
Contributor Author

Ulexus commented Mar 13, 2017

Yeah, sorry I haven't had time to work on this more. If you've a better idea, please take it where you want!

@akshayjshah
Copy link
Contributor

No worries at all - appreciate the work that you've already put in!

Ulexus and others added 3 commits March 13, 2017 10:02
If call to `Sync()` fails, wrap the WriteSyncer with a fake `Sync()`.
Fixes uber-go#328
@akshayjshah akshayjshah requested a review from prashantv March 13, 2017 17:05
@akshayjshah akshayjshah changed the title Test Sync() call when calling AddSync Ignore Sync errors Mar 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

write error: sync /dev/stdout: invalid argument
6 participants