Repository navigation
Proposal: move node-report into core #18760
Description
The idea of moving node-report was brought up at the Diagnostics Summit.
Thoughts? Objections?
The idea of moving node-report was brought up at the Diagnostics Summit.
Thoughts? Objections?
Activity
dshaw commentedon Feb 13, 2018
Also: https://github.com/nodejs/node-report
gireeshpunathil commentedon Feb 13, 2018
+1.
By moving this into core, will the report generation hooks be enabled by default?
targos commentedon Feb 13, 2018
+1
mmarchini commentedon Feb 13, 2018
+1
jasnell commentedon Feb 13, 2018
+1
richardlau commentedon Feb 13, 2018
Some previous discussion in nodejs/node-report#103.
Fishrock123 commentedon Feb 13, 2018
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.
jasnell commentedon Feb 13, 2018
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.
devsnek commentedon Feb 13, 2018
+1
hiroppy commentedon Feb 13, 2018
+1
joyeecheung commentedon Feb 14, 2018
@gireeshpunathil
I think what we have talked about at the summit was, first putting
triggerReport()inutiland 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, .etcjoyeecheung commentedon Feb 14, 2018
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
rnchamberlain commentedon Feb 14, 2018
@joyeecheung there is a CI here: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/
richardlau commentedon Feb 14, 2018
In addition to the CI for testing node-report itself it is also included in CITGM runs.
hashseed commentedon Feb 14, 2018
+1
Trott commentedon Nov 6, 2018
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.
gireeshpunathil commentedon Nov 7, 2018
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.
gireeshpunathil commentedon Jan 18, 2019
#22712 is merged now; so closing. fyi - @nodejs/diagnostics