The idea of moving node-report was brought up at the Diagnostics Summit.
Thoughts? Objections?
+1.
By moving this into core, will the report generation hooks be enabled by default?
+1
+1
+1
Some previous discussion in https://github.com/nodejs/node-report/issues/103.
I'd be ok with this but it will need to modifications to default behavior and also perhaps other things.
Also, the api would have to be not exposed as a separate module. I propose we do not attempt to expose the JS api until the other parts are implemented.
I agree with not exposing the js API as a separate module. A function off util or console should work fine. There are definitely a few internal bits that will need to be looked at also.
+1
+1
@gireeshpunathil
By moving this into core, will the report generation hooks be enabled by default?
I think what we have talked about at the summit was, first putting triggerReport() in util and making it a semver-minor change, the whole thing will be experimental. Then we iterate on that and discuss whether to enable it by default, where it should log to and how to specify that, .etc
One thing we need to discuss: how is the test CI integration of node-report going to work? Right now it does not have a testing CI I believe. During the summit we talked about vendoring it in like node-inspect, so it would be great if it has separate testing CIs for PRs of its own.
cc @richardlau @rnchamberlain @hhellyer
@joyeecheung there is a CI here: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/
In addition to the CI for testing node-report itself it is also included in CITGM runs.
+1
It seems like perhaps this should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. (Or, perhaps better, open a PR or get the existing one at #22712 moving.) I'm just tidying up and not acting on a super-strong opinion or anything like that.
22712 is on track, just that I am low on bandwidth at the moment, went with some open libuv issues that were causing CI failures. 22712 should come to life soon.
Most helpful comment
@joyeecheung there is a CI here: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/