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

Improve writing across multiple files. #245

Merged
merged 4 commits into from Nov 5, 2014
Merged

Conversation

@arb
Copy link
Contributor

arb commented Nov 5, 2014

Ran this against my local test server and while the writing isn't "even", neither file seems starved.

Fixes #245
Fixes #244

@arb arb added this to the 2.5.1 milestone Nov 5, 2014
@arb arb assigned geek Nov 5, 2014
@arb arb added the bug label Nov 5, 2014
@@ -390,15 +390,22 @@ internals.Monitor.prototype._broadcastFile = function () {
}
};

for (var i = 0; i < keysLength; ++i) {
Async.each(keys, function (key, callback) {

This comment has been minimized.

Copy link
@geek

geek Nov 5, 2014

Member

Usually use next for async functions instead of callback: https://github.com/hapijs/hapi/blob/master/lib/request.js#L292-L309


// Empty queue (must not set to [] or queue reference will change)
subscriberQueue.length = 0;
self._logToFile(key, events, function(err) {

This comment has been minimized.

Copy link
@geek

geek Nov 5, 2014

Member

function (err) {

if (err) {
console.error(err);
}
callback(null);

This comment has been minimized.

Copy link
@geek

geek Nov 5, 2014

Member

no need to pass null

@@ -687,7 +694,7 @@ internals.Monitor.prototype._logToFile = function (dest, events, callback) {
return logNext();
}

This comment has been minimized.

Copy link
@geek

geek Nov 5, 2014

Member

Please also move the try block into a new function so that logToFile can be optimized

geek added a commit that referenced this pull request Nov 5, 2014
Improve writing across multiple files.
@geek geek merged commit 4fc51a5 into hapijs:branch2.5.1 Nov 5, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.