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.

Sign up for GitHub

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

Jump to bottom

src: add errorProperties on process.report #28426

Closed
wants to merge 2 commits into from
Closed

src: add errorProperties on process.report #28426

wants to merge 2 commits into from
+137 −38

Conversation

himself65
Copy link
Member

@himself65 himself65 commented Jun 25, 2019 •
edited
Loading

fix #28415

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • 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 Jun 25, 2019
@mmarchini
Copy link
Contributor

Thank you for working on this!

I wonder if this should be implemented on C++ instead though (using Object::GetOwnPropertyNames + Object::Get). Don't have strong opinions, but it would be nice to have the properties as a JSON object instead of a string though (and implementing in C++ would allow that).

@himself65 himself65 force-pushed the 28415 branch 3 times, most recently from eb4e7cf to b236e8c Compare June 26, 2019 02:48
@himself65 himself65 changed the title src: add errorProperties on writeReport src: add errorProperties on process.report Jun 26, 2019
@himself65 himself65 force-pushed the 28415 branch 6 times, most recently from 5e0527b to b7083e2 Compare June 26, 2019 07:53
@ZYSzys ZYSzys added the wip Issues and PRs that are still a work in progress. label Jun 26, 2019
@himself65 himself65 force-pushed the 28415 branch 2 times, most recently from 0e8af53 to 7770f06 Compare June 26, 2019 09:36
@himself65

This comment has been minimized.

Sign in to view
@himself65 himself65 marked this pull request as ready for review June 28, 2019 13:35
@himself65

This comment has been minimized.

Sign in to view
@himself65 himself65 force-pushed the 28415 branch 2 times, most recently from 978f30e to 6a2f2e9 Compare June 28, 2019 13:53
@himself65 himself65 mentioned this pull request Jun 28, 2019
Closed
@himself65 himself65 force-pushed the 28415 branch 7 times, most recently from 740ad5b to 919eba6 Compare June 29, 2019 04:03
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32078/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32090/

@github-actions github-actions bot mentioned this pull request Jun 26, 2020
Open
13 tasks
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32100/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32101/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32102/

@jasnell jasnell added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed review wanted PRs that need reviews. labels Jun 26, 2020
jasnell pushed a commit that referenced this pull request Jun 26, 2020
@himself65 @jasnell
src: add errorProperties on process.report
870f0fc 
PR-URL: #28426
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@jasnell
Copy link
Member

jasnell commented Jun 26, 2020

Landed in 870f0fc

@jasnell jasnell closed this Jun 26, 2020
@himself65 himself65 deleted the 28415 branch June 26, 2020 17:12
@github-actions github-actions bot mentioned this pull request Jun 27, 2020
Open
11 tasks
codebytere pushed a commit that referenced this pull request Jun 27, 2020
@himself65 @codebytere
src: add errorProperties on process.report
9760c6c 
PR-URL: #28426
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@github-actions github-actions bot mentioned this pull request Jun 28, 2020
Open
15 tasks
@codebytere codebytere mentioned this pull request Jun 28, 2020
Merged
This was referenced Jun 29, 2020
Open
Open
codebytere pushed a commit that referenced this pull request Jun 30, 2020
@himself65 @codebytere
src: add errorProperties on process.report
3fe1cee 
PR-URL: #28426
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@github-actions github-actions bot mentioned this pull request Jul 1, 2020
Open
27 tasks
codebytere pushed a commit that referenced this pull request Jul 10, 2020
@himself65 @codebytere
src: add errorProperties on process.report
5efbed0 
PR-URL: #28426
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
codebytere pushed a commit that referenced this pull request Jul 12, 2020
@himself65 @codebytere
src: add errorProperties on process.report
d3737a1 
PR-URL: #28426
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@codebytere codebytere mentioned this pull request Jul 13, 2020
Merged
codebytere pushed a commit that referenced this pull request Jul 14, 2020
@himself65 @codebytere
src: add errorProperties on process.report
bde3fb2 
PR-URL: #28426
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@addaleax addaleax addaleax approved these changes

@Trott Trott Trott approved these changes

@jasnell jasnell jasnell approved these changes

@BridgeAR BridgeAR Awaiting requested review from BridgeAR

@mmarchini mmarchini Awaiting requested review from mmarchini

Assignees
No one assigned
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. report Issues and PRs related to process.report.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Diagnostics report: inspect the error object