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

Multiple reporters and multiple outputs #389

Merged
merged 5 commits into from Jul 2, 2015
Merged

Conversation

@hofan41
Copy link
Contributor

hofan41 commented Jul 1, 2015

Closes #285

@hofan41

This comment has been minimized.

Copy link
Contributor Author

hofan41 commented on lib/reporters/multiple.js in 861f576 Jul 1, 2015

Typo here need to fix.

@@ -21,6 +21,10 @@ internals.protos = {

internals.requireReporter = function (reporter) {

if (Array.isArray(reporter)) {
return require('./multiple');

This comment has been minimized.

Copy link
@geek

geek Jul 1, 2015

Member

Add a comment to the top of the file that indicates that multiple is required further down. Just easier to look at the module section to find dependencies

// ('./multiple') loaded below
// Load modules

var Path = require('path');
var Reporters = require('./index');

This comment has been minimized.

Copy link
@geek

geek Jul 1, 2015

Member

Please move Reporters to be required last in the list

Path
Hoek
Reporters

We try to do:

Node core
3rd Party
Local
@geek

This comment has been minimized.

Copy link
Member

geek commented Jul 1, 2015

@hofan41 Please update the readme with information on how to use this enhancement

@geek geek added the feature label Jul 1, 2015
@geek geek self-assigned this Jul 1, 2015
hofan41 added 2 commits Jul 1, 2015
@geek

This comment has been minimized.

Copy link
Member

geek commented Jul 2, 2015

@hofan41 really well done, this is awesome!

@geek geek added this to the 5.12.0 milestone Jul 2, 2015
geek added a commit that referenced this pull request Jul 2, 2015
Multiple reporters and multiple outputs
@geek geek merged commit 83acb83 into hapijs:master Jul 2, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hofan41

This comment has been minimized.

Copy link
Contributor Author

hofan41 commented Jul 2, 2015

Glad I could help! 👍

@hofan41 hofan41 deleted the hofan41:rebase-9223fd8 branch Jul 3, 2015
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.