-
Notifications
You must be signed in to change notification settings - Fork 88
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
Reduce file descriptor consumption #59
Conversation
fileHandle?.seekToEndOfFile() | ||
// prepare for the next entry | ||
if newFile == false { | ||
fileHandle?.write(",".data(using: .utf8)!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we absolutely know 100% of the time this explicit will work on all platforms? Makes me uneasy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would it not work on all platforms?? It's almost identical to what it was before and tests are passing hunky dory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it just being a comma should be okay.
assert(false, "Storage: failed to write event to \(storeFile), error: \(error)") | ||
// write the data | ||
fileHandle?.write(jsonData) | ||
if #available(tvOS 13, *) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be good to add a comment why we are doing this for 13 and above but supporting 11.
Reduces file descriptor usage by storing file handle rather than opening a new one with every write.