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

report: move diagnostic reports to stable #32242

Merged
merged 6 commits into from
Mar 15, 2020
Merged

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Mar 13, 2020

This PR promotes the experimental diagnostic report feature to stable, as outlined in #26293 (comment).

Fixes: #26293

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Mar 13, 2020
@richardlau richardlau added the baking-for-lts PRs that need to wait before landing in a LTS release. label Mar 13, 2020
@gireeshpunathil
Copy link
Member

@cjihrig :

One of the item in the referenced issue is Make the --report-on-fatalerror CLI flag experimental , due to a bug in that API. #32207 is in place to address that, just an FYI.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@cjihrig
Copy link
Contributor Author

cjihrig commented Mar 14, 2020

The CI is fine, except test-memory-usage is failing on ARM 12. #32239 might help?

@cjihrig cjihrig added the semver-major PRs that contain breaking changes and should be released in the next major version. label Mar 14, 2020
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 15, 2020

@cjihrig cjihrig added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 15, 2020
@cjihrig
Copy link
Contributor Author

cjihrig commented Mar 15, 2020

This can land, but it needs another review from @nodejs/tsc.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

This commit removes all #ifdef NODE_REPORT checks in the src
directory.

PR-URL: nodejs#32242
Fixes: nodejs#26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
PR-URL: nodejs#32242
Fixes: nodejs#26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
This commit makes the configure --without-report flag a no-op.
This commit also updates a test that depends on the report CLI
flags being conditionally present.

PR-URL: nodejs#32242
Fixes: nodejs#26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
The report feature won't ever be disabled moving forward, so
checking for its existence in the tests is no longer needed.

PR-URL: nodejs#32242
Fixes: nodejs#26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
This commit makes the --experimental-report CLI flag a no-op.

PR-URL: nodejs#32242
Fixes: nodejs#26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
This commit updates the stability documentation for the report
feature. All diagnostic report APIs are now listed as stable,
with the exception of report-on-fatalerror, which still has a
few bugs to work out.

PR-URL: nodejs#32242
Fixes: nodejs#26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@cjihrig cjihrig merged commit a022d38 into nodejs:master Mar 15, 2020
@cjihrig cjihrig deleted the report branch March 15, 2020 17:31
@cjihrig
Copy link
Contributor Author

cjihrig commented Mar 15, 2020

Landed in 8617508...a022d38. Thanks for the reviews!

MylesBorins added a commit that referenced this pull request Mar 24, 2020
Notable changes:

* build:
  * macOS package notarization (Rod Vagg)
    #31459
* deps:
  * update npm to 6.14.3 (Myles Borins)
    #32368
  * update to uvwasi 0.0.6 (Colin Ihrig) [#32309](#32309)
  * upgrade to libuv 1.35.0 (Colin Ihrig)
    #32204
* node\_report:
  * move diagnostic reports to stable (Colin Ihrig)
    #32242
* worker:
  * allow URL in Worker constructor (Antoine du HAMEL)
    #31664
* util:
  * use a global symbol for `util.promisify.custom` (ExE Boss)
    #31672

PR-URL: #32376
MylesBorins added a commit that referenced this pull request Mar 24, 2020
Notable changes:

* build:
  * macOS package notarization (Rod Vagg)
    #31459
* deps:
  * update npm to 6.14.3 (Myles Borins)
    #32368
  * update to uvwasi 0.0.6 (Colin Ihrig) [#32309](#32309)
  * upgrade to libuv 1.35.0 (Colin Ihrig)
    #32204
* node\_report:
  * move diagnostic reports to stable (Colin Ihrig)
    #32242
* worker:
  * allow URL in Worker constructor (Antoine du HAMEL)
    #31664
* util:
  * use a global symbol for `util.promisify.custom` (ExE Boss)
    #31672

PR-URL: #32376
MylesBorins added a commit that referenced this pull request Mar 25, 2020
macOS package notarization and a change in builder configuration:

The macOS binaries for this release, and future 13.x releases, are now
being compiled on macOS 10.15 (Catalina) with Xcode 11 to support
package notarization, a requirement for installing on .pkg files on
macOS 10.15 and later. Previous builds of Node.js 13.x were compiled on
macOS 10.11 (El Capitan) with Xcode 10. As binaries are still being
compiled to support a minimum of macOS 10.10 (Yosemite) we do not
anticipate this having a negative impact on Node.js 13.x users with
older versions of macOS.

Notable changes:

* build:
  * macOS package notarization (Rod Vagg)
    #31459
* deps:
  * update npm to 6.14.3 (Myles Borins)
    #32368
  * update to uvwasi 0.0.6 (Colin Ihrig)
    #32309
  * upgrade to libuv 1.35.0 (Colin Ihrig)
    #32204
* lib:
  * add --disable-proto option to cli (Gus Caplan)
    #32279
* node_report:
  * move diagnostic reports to stable (Colin Ihrig)
    #32242
* worker:
  * allow URL in Worker constructor (Antoine du HAMEL)
    #31664
* util:
  * use a global symbol for `util.promisify.custom` (ExE Boss)
    #31672

PR-URL: #32376
MylesBorins added a commit that referenced this pull request Mar 26, 2020
macOS package notarization and a change in builder configuration:

The macOS binaries for this release, and future 13.x releases, are now
being compiled on macOS 10.15 (Catalina) with Xcode 11 to support
package notarization, a requirement for installing on .pkg files on
macOS 10.15 and later. Previous builds of Node.js 13.x were compiled on
macOS 10.11 (El Capitan) with Xcode 10. As binaries are still being
compiled to support a minimum of macOS 10.10 (Yosemite) we do not
anticipate this having a negative impact on Node.js 13.x users with
older versions of macOS.

Notable changes:

* build:
  * macOS package notarization (Rod Vagg)
    #31459
* deps:
  * upgrade npm to 6.14.4 (Ruy Adorno)
    #32495
  * update to uvwasi 0.0.6 (Colin Ihrig)
    #32309
  * upgrade to libuv 1.35.0 (Colin Ihrig)
    #32204
* lib:
  * add --disable-proto option to cli (Gus Caplan)
    #32279
* node_report:
  * move diagnostic reports to stable (Colin Ihrig)
    #32242
* worker:
  * allow URL in Worker constructor (Antoine du HAMEL)
    #31664
* util:
  * use a global symbol for `util.promisify.custom` (ExE Boss)
    #31672

PR-URL: #32376
MylesBorins added a commit that referenced this pull request Mar 26, 2020
macOS package notarization and a change in builder configuration:

The macOS binaries for this release, and future 13.x releases, are now
being compiled on macOS 10.15 (Catalina) with Xcode 11 to support
package notarization, a requirement for installing on .pkg files on
macOS 10.15 and later. Previous builds of Node.js 13.x were compiled on
macOS 10.11 (El Capitan) with Xcode 10. As binaries are still being
compiled to support a minimum of macOS 10.10 (Yosemite) we do not
anticipate this having a negative impact on Node.js 13.x users with
older versions of macOS.

Notable changes:

* build:
  * macOS package notarization (Rod Vagg)
    #31459
* deps:
  * upgrade npm to 6.14.4 (Ruy Adorno)
    #32495
  * update to uvwasi 0.0.6 (Colin Ihrig)
    #32309
  * upgrade to libuv 1.35.0 (Colin Ihrig)
    #32204
* lib:
  * add --disable-proto option to cli (Gus Caplan)
    #32279
* node_report:
  * move diagnostic reports to stable (Colin Ihrig)
    #32242
* worker:
  * allow URL in Worker constructor (Antoine du HAMEL)
    #31664
* util:
  * use a global symbol for `util.promisify.custom` (ExE Boss)
    #31672

PR-URL: #32376
MylesBorins added a commit that referenced this pull request Mar 26, 2020
macOS package notarization and a change in builder configuration:

The macOS binaries for this release, and future 13.x releases, are now
being compiled on macOS 10.15 (Catalina) with Xcode 11 to support
package notarization, a requirement for installing on .pkg files on
macOS 10.15 and later. Previous builds of Node.js 13.x were compiled on
macOS 10.11 (El Capitan) with Xcode 10. As binaries are still being
compiled to support a minimum of macOS 10.10 (Yosemite) we do not
anticipate this having a negative impact on Node.js 13.x users with
older versions of macOS.

Notable changes:

* build:
  * macOS package notarization (Rod Vagg)
    #31459
* deps:
  * upgrade npm to 6.14.4 (Ruy Adorno)
    #32495
  * update to uvwasi 0.0.6 (Colin Ihrig)
    #32309
  * upgrade to libuv 1.35.0 (Colin Ihrig)
    #32204
* lib:
  * add --disable-proto option to cli (Gus Caplan)
    #32279
* node_report:
  * move diagnostic reports to stable (Colin Ihrig)
    #32242
* worker:
  * allow URL in Worker constructor (Antoine du HAMEL)
    #31664
* util:
  * use a global symbol for `util.promisify.custom` (ExE Boss)
    #31672

PR-URL: #32376
@targos targos removed the baking-for-lts PRs that need to wait before landing in a LTS release. label Apr 25, 2020
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
This commit removes all #ifdef NODE_REPORT checks in the src
directory.

PR-URL: nodejs#32242
Fixes: nodejs#26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
PR-URL: nodejs#32242
Fixes: nodejs#26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
This commit makes the configure --without-report flag a no-op.
This commit also updates a test that depends on the report CLI
flags being conditionally present.

PR-URL: nodejs#32242
Fixes: nodejs#26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
The report feature won't ever be disabled moving forward, so
checking for its existence in the tests is no longer needed.

PR-URL: nodejs#32242
Fixes: nodejs#26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
This commit makes the --experimental-report CLI flag a no-op.

PR-URL: nodejs#32242
Fixes: nodejs#26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
This commit updates the stability documentation for the report
feature. All diagnostic report APIs are now listed as stable,
with the exception of report-on-fatalerror, which still has a
few bugs to work out.

PR-URL: nodejs#32242
Fixes: nodejs#26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
This commit removes all #ifdef NODE_REPORT checks in the src
directory.

PR-URL: #32242
Fixes: #26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
PR-URL: #32242
Fixes: #26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
This commit makes the configure --without-report flag a no-op.
This commit also updates a test that depends on the report CLI
flags being conditionally present.

PR-URL: #32242
Fixes: #26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
The report feature won't ever be disabled moving forward, so
checking for its existence in the tests is no longer needed.

PR-URL: #32242
Fixes: #26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
This commit makes the --experimental-report CLI flag a no-op.

PR-URL: #32242
Fixes: #26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
This commit updates the stability documentation for the report
feature. All diagnostic report APIs are now listed as stable,
with the exception of report-on-fatalerror, which still has a
few bugs to work out.

PR-URL: #32242
Fixes: #26293
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
codebytere added a commit to electron/electron that referenced this pull request May 28, 2020
codebytere added a commit to electron/electron that referenced this pull request Jun 1, 2020
jkleinsc pushed a commit to electron/electron that referenced this pull request Jun 8, 2020
jkleinsc pushed a commit to electron/electron that referenced this pull request Jun 9, 2020
codebytere added a commit to electron/electron that referenced this pull request Jun 15, 2020
codebytere added a commit to electron/electron that referenced this pull request Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discussion: Criteria for upgrading diagnostic reporting from experimental to stable
9 participants