-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
doc: elevate diagnostic report to tier1 #32732
Conversation
diagnostic report qualifies for all the criteria for being in tier1. Classify it as such.
@@ -123,7 +124,6 @@ The tools are currently assigned to Tiers as follows: | |||
|
|||
| Tool Type | Tool/API Name | Regular Testing in Node.js CI | Integrated with Node.js | Target Tier | | |||
|-----------|---------------------------|-------------------------------|-------------------------|-------------| | |||
| FFDC | node-report | No | No | 1 | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather either keep the npm node-report
module as unclassified or moved to tier 4.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move it to tier unclassified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@richardlau @mhdawson - reverted the case of node-report
, PTAL!
For diagnostics report, since it's in core the tests for it are part of the regular test CI so no concerns there. I'm not sure the Release WG has been consulted on the broader Tier requirements. The only mechanism we have of running tests for releases is either (a) tests that exist in core and are therefore run as part of regular/nightly CI, or (b) CITGM (which can only test npms). I'm hesitant to place more requirements in the future on @nodejs/releasers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@richardlau agreed, for those tools to be tier 1 they need to be tested either on Node.js core or as part of CITGM. We should add this as a pre-requirement for Tier 1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
diagnostic report qualifies for all the criteria for being in tier1. Classify it as such. PR-URL: #32732 Refs: nodejs/diagnostics#369 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>
Landed in 6ad65ae |
diagnostic report qualifies for all the criteria for being in tier1. Classify it as such. PR-URL: #32732 Refs: nodejs/diagnostics#369 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>
diagnostic report qualifies for all the criteria for being in tier1. Classify it as such. PR-URL: #32732 Refs: nodejs/diagnostics#369 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>
diagnostic report qualifies for all the criteria for being in tier1. Classify it as such. PR-URL: #32732 Refs: nodejs/diagnostics#369 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>
diagnostic report qualifies for all the criteria for being in tier1. Classify it as such. PR-URL: #32732 Refs: nodejs/diagnostics#369 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>
diagnostic WG recommends that: diagnostic report qualifies for all the criteria for being in tier1. Classify it as such.
checklist:
this is the first time a tool is potentially entering the
tier1
status. As per the documented process, the most notable one is:A release will not be shipped if the test suite for the tool/API is not green.
(this page, second para). So alerting the release team for their awareness and / or review.@richardlau raised a point about retaining
node-report
in theunclassified
list. Whilediagnostic report
is replacement ofnode-report
in principle, in reality both need to co-exist untilv10.x
goes out of support (04/21). I am fine in either way, but my opinion is toreplace
node-report
in the list -node-report
is an npm module, so its support tier status listing in core asunclassified
or absence of it may not be too critical to the ecosystem.Refs: nodejs/diagnostics#369
/cc @nodejs/releasers
/cc @nodejs/diagnostics
/cc @nodejs/tooling
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes