Improve README.md

[Fishrock123]

Better code blocks, clarity, markdown formatting.

R=@rnchamberlain

[rnchamberlain]

Thanks for the improvements, LGTM

Note that I was persuaded in #2 not to enable the exception and error hooks and the signal handler by default. So the node -r nodereport my-script.js usage won't do anything unless the user either calls require('nodereport') and nodereport.setEvents(), or sets the NODEREPORT_EVENTS env var.

[Fishrock123]

... what? That honestly renders this tool unusable a lot of the time IMO. Is there no other way to enable it?

[sam-github]

@Fishrock123 modules that when required auto-hook themselves into the system, without the control of the user, are horrid.

Can you please explain:

renders this tool unusable a lot of the time

Specifically, why

require('nodereport').setEvents()

is "unusable", and

require('nodereport')

by implication is useable. unusable is a strong word for having to type a half-dozen more characters!

Btw, my suggested usage was

require('nodereport')()

to enable the default hooks, and

require('nodereport')({... options...})

for customization.

@rnchamberlain had trouble implementing that in C++. I'm not sure if that is an inherent limitation of the addon API or not, but I also don't see why the nodereport package.main has to be C++, why not have a half-dozen line js wrapper that exports a function?

[sam-github]

Ah, I see, you hope to use it like:

node -r nodereport my-script.js

Fair enough, except that usage breaks this usage:

require('nodereport').triggerReport()

Where absolutely no hidden side effects (like changing the behaviour of the node process on uncaught exceptions) is desired

I suggest the behaviours be distinguished via require path, either:

node -r nodereport/attach my-script.js

or

require('nodereport/noattach').triggerReport()

or both.

[mhdawson]

I definitely would like the option for people to be able to enable without code change, along with default behavior. Also agree that for a normal require in code we don't want the auto-hook.

Is it possible to have it be:

require('nodereport') for the case without auto-hook

and required('nodereport/attach') or node -r nodereport/attach for the hook case.

It would be nice to have the default expected module behaviour for a module without having to know you need to do something special while at the same time giving the user a mechanism to full enable when they know they want to.

[Fishrock123]

I expect to have those "side-effects" with a module like this... 😐

[Fishrock123]

Also yes, I want to never directly modify edit the actual code that I'm running.

[rnchamberlain]

So right now you can do:

$ export NODEREPORT_EVENTS=exception+fatalerror+signal+apicall
$ node -r nodereport myapp.js

and all the functionality is there. But setting the env vars is nasty. For the sys admin folks I'd prefer this worked out-of-the-box, simply, without any user code change. To accommodate Sam's use case as well (which is perfectly valid - a more advanced user wanting to code to .triggerReport() without the side effects) I think we need something like
var nr = require('nodereport')('no hooks please, unless the user already requested them')

I have not yet worked out how to implement that - i.e extra parameters on the module initialize().

For the purposes of landing this PR maybe we should remove this line for the time being, or alternatively add the nasty env vars line above it :
node -r nodereport my-script.js

[sam-github]

Yes, @Fishrock123 I suggest you modify the README to reflect how it actually works for now so this can land, and discussion of how it could work move to #25

[Fishrock123]

Updated.

[mhdawson]

LGTM

[rnchamberlain]

Landed as 9b80f24