GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — src: Handle NULL env scenario by HarshithaKP · Pull Request #31899 · nodejs/node

Conversation

@HarshithaKP

@HarshithaKP HarshithaKP commented Feb 21, 2020

Copy link
Copy Markdown
Member

Convert hard assertion into a throw with a useful error
message in src/module_wrap.cc

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Feb 21, 2020
addaleax
addaleax reviewed Feb 21, 2020
View reviewed changes
Comment thread src/module_wrap.cc Resolved

Uh oh!

There was an error while loading. Please reload this page.
@HarshithaKP
HarshithaKP force-pushed the module_wrap_handle_null_ptr branch from c06e445 to baa5f1a Compare February 21, 2020 08:39
mscdex
mscdex reviewed Feb 21, 2020
View reviewed changes
Comment thread src/node_errors.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
@HarshithaKP

HarshithaKP commented Feb 21, 2020

Copy link
Copy Markdown
Member Author

While going through the file found one more similar location, fixed it.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@HarshithaKP

HarshithaKP commented Feb 21, 2020

Copy link
Copy Markdown
Member Author

Surprisingly make format-cpp did not fix the error. Fixed it in one more commit.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
addaleax
addaleax approved these changes Feb 22, 2020
View reviewed changes

@addaleax addaleax left a comment

Copy link
Copy Markdown
Member

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 with docs added

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/node_errors.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
addaleax
addaleax reviewed Mar 4, 2020
View reviewed changes
Comment thread doc/api/errors.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/node_errors.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
@HarshithaKP
HarshithaKP force-pushed the module_wrap_handle_null_ptr branch from 870a3cb to 53c0050 Compare March 5, 2020 07:17
@HarshithaKP
src: Handle NULL env scenario
8069174 
Convert hard assertion into a throw with a useful error
message in src/module_wrap.cc
@HarshithaKP
HarshithaKP force-pushed the module_wrap_handle_null_ptr branch from 8594430 to 8069174 Compare March 5, 2020 07:22
@HarshithaKP

HarshithaKP commented Mar 5, 2020

Copy link
Copy Markdown
Member Author

I rebased and commits squashed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Mar 5, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/29562/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
addaleax
addaleax approved these changes Mar 5, 2020
View reviewed changes
Comment thread src/node_errors.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
@HarshithaKP
fixup: address review comment
a2b8310
@addaleax addaleax added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Mar 5, 2020
@nodejs-github-bot

nodejs-github-bot commented Mar 5, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/29563/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@addaleax

addaleax commented Mar 9, 2020

Copy link
Copy Markdown
Member

Landed in e9fa5ae
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
addaleax pushed a commit that referenced this pull request Mar 9, 2020
@HarshithaKP @addaleax
src: handle NULL env scenario
e9fa5ae 
Convert hard assertion into a throw with a useful error
message in src/module_wrap.cc.

PR-URL: #31899
Reviewed-By: Anna Henningsen <[email protected]>
@addaleax addaleax closed this Mar 9, 2020
MylesBorins pushed a commit that referenced this pull request Mar 9, 2020
@HarshithaKP @MylesBorins
src: handle NULL env scenario
cba75c5 
Convert hard assertion into a throw with a useful error
message in src/module_wrap.cc.

PR-URL: #31899
Reviewed-By: Anna Henningsen <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Mar 10, 2020
v13.11.0 proposal #32185
Merged
@targos targos added backport-blocked-v12.x and removed author ready PRs with CI started, the required approvals, and no outstanding review comments. backport-blocked-v12.x labels Apr 20, 2020
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
@HarshithaKP @targos
src: handle NULL env scenario
b3fb4f3 
Convert hard assertion into a throw with a useful error
message in src/module_wrap.cc.

PR-URL: nodejs#31899
Reviewed-By: Anna Henningsen <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
@HarshithaKP @targos
src: handle NULL env scenario
51a3456 
Convert hard assertion into a throw with a useful error
message in src/module_wrap.cc.

PR-URL: #31899
Reviewed-By: Anna Henningsen <[email protected]>
@targos targos mentioned this pull request May 2, 2020
v12.17.0 release proposal #33197
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax approved these changes
+1 more reviewer
@mscdex mscdex mscdex left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@HarshithaKP @nodejs-github-bot @addaleax @mscdex @targos
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.