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

Migrate to use pipes and streams. #33

Merged
merged 4 commits into from Feb 3, 2015

Conversation

@arb
Copy link
Contributor

arb commented Feb 2, 2015

Related to hapijs/good#292

@arb arb added the breaking changes label Feb 2, 2015
@arb arb added this to the 5.0.0 milestone Feb 2, 2015
@cjihrig cjihrig assigned arb and unassigned cjihrig Feb 2, 2015
@arb arb assigned cjihrig and unassigned arb Feb 2, 2015
if (event === 'response') {
return this._formatResponse(eventData, tags);
if (!stream._readableState.objectMode) {
return callback(new Error('stream must be in objectMode'));

This comment has been minimized.

Copy link
@cjihrig

cjihrig Feb 2, 2015

Contributor

Maybe say object mode instead of objectMode

return this._printEvent(eventPrintData);
}
var eventName = data.event;
var tags = (data.tags || []).concat([]);

This comment has been minimized.

Copy link
@cjihrig

cjihrig Feb 2, 2015

Contributor

Any chance that data.tags won't be an array? If so, I'd use Array.isArray() here.

This comment has been minimized.

Copy link
@arb

arb Feb 2, 2015

Author Contributor

So that line should be able to deal with any non-arrays by concatenating them into an empty array. It's unlikely that they won't be an array, but if data.tags isn't, that line should coalesce them into one.

var timestring = m.format(this._settings.format);
if (eventName === 'ops') {
eventPrintData.data = Hoek.format('memory: %sMb, uptime (seconds): %s, load: %s',
Math.round(data.proc.mem.rss / (1024 * 1024)),

This comment has been minimized.

Copy link
@cjihrig

cjihrig Feb 2, 2015

Contributor

Should these lines be indented more?

@arb

This comment has been minimized.

Copy link
Contributor Author

arb commented Feb 2, 2015

@cjihrig made requested updates.

@arb arb force-pushed the arb:pipe-update branch from 9ad6348 to 92c7513 Feb 2, 2015
arb added 2 commits Feb 3, 2015
cjihrig added a commit that referenced this pull request Feb 3, 2015
Migrate to use pipes and streams.
@cjihrig cjihrig merged commit 601f03c into hapijs:master Feb 3, 2015
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
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.