dockershim: check if f.Sync() returns an error and surface it #52125
Conversation
yujuhong
commented
Sep 7, 2017
|
/approve no-issue |
| f.Sync() | ||
| if err == nil { | ||
| // Only sync if the Write completed successfully. | ||
| err = f.Sync() |
There was a problem hiding this comment.
Is there a reason for hiding old errors behind new errors? If it's so control flow makes it to f.Close(), perhaps defer is better?
There was a problem hiding this comment.
This function surfaces the errors in the following order:
- write error
- sync error
- close error
If (1) is non-nil, return (1). If (2) is non-nil, return (2), etc.
This follows the existing flow and convention in ioutil.WriteFile()
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: freehan, yujuhong Associated issue requirement bypassed by: yujuhong The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue (batch tested with PRs 52339, 52343, 52125, 52360, 52301) |