serialfile: localize os.Open into NewSerialFile #1629
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rht
force-pushed
the
localize-os-open
branch
2 times, most recently
from
0d4d18d
to
9219fc1
Compare
| @@ -42,21 +36,11 @@ func newSerialFile(name, path string, file *os.File, stat os.FileInfo) (File, er | |||
|
|
|||
| // for directories, stat all of the contents first, so we know what files to | |||
| // open when NextFile() is called | |||
| contents, err := file.Readdir(0) | |||
| contents, err := ioutil.ReadDir(path) | |||
There was a problem hiding this comment.
why change this? we already have the file. no need to make an extra syscall
There was a problem hiding this comment.
What if os.Open() is done only when !stat.IsDir()?
There was a problem hiding this comment.
Not possible because in NextFile(), f.Close() requires the opened os.File object.
There was a problem hiding this comment.
Still possible but then NextFile() no longer uses newSerialFile().
rht
force-pushed
the
localize-os-open
branch
2 times, most recently
from
3ed13af
to
0d36141
Compare
|
LGTM. @whyrusleeping ? |
|
I think this needs rebasing on top of symlink PR |
rht
force-pushed
the
localize-os-open
branch
from
0d36141
to
9b48ede
Compare
rht
force-pushed
the
localize-os-open
branch
from
9b48ede
to
6ff746f
Compare
|
Rebased |
|
that goprocess issue is happening more and more frequently. Otherwise, this LGTM |
License: MIT Signed-off-by: rht <[email protected]>
License: MIT Signed-off-by: rht <[email protected]>
rht
force-pushed
the
localize-os-open
branch
from
6ff746f
to
0c95ed6
Compare
jbenet
added a commit
that referenced
this pull request
Sep 3, 2015
serialfile: localize os.Open into NewSerialFile
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.