Ignore close errors in read-only files. #1130
Conversation
pvdrz
force-pushed
the
ignore-close-read-only
branch
from
f2783db to
aec2f31
Compare
src/shims/fs.rs
Outdated
| // And return the result. | ||
| result | ||
| // We sync the file if it was opened in a mode different than read-only. | ||
| if !handle.read_only { |
There was a problem hiding this comment.
Unless there are good arguments for the opposite, it's better to put the positive case first (if handle.read_only). That avoids a mental double-negation when considering the else branch.
There was a problem hiding this comment.
I did it this way to keep the "usual" behavior first and then be able to explain why the read-only exception is required.
There was a problem hiding this comment.
Why is read-write more "usual" than read-only?
There was a problem hiding this comment.
huh that's a good question. I don't think that's the case. Taking that into account, should I change the field inside the handle to read-write then?
There was a problem hiding this comment.
Whatever you prefer, as long as you avoid the negation here. ;)
There was a problem hiding this comment.
I changed it to avoid the double negation.
pvdrz
force-pushed
the
ignore-close-read-only
branch
from
aec2f31 to
ce4e1f9
Compare
|
Looking good, thanks! @bors r+ |
|
📌 Commit a40a99d has been approved by |
|
☀️ Test successful - checks-travis, status-appveyor |
this fixes #999
r? @RalfJung