Skip to content

Conversation

@hidecology
Copy link
Contributor

Fixes: #30872

@nodejs-github-botnodejs-github-bot added assert Issues and PRs related to the assert subsystem. needs-ci PRs that need a full CI run. labels Feb 22, 2023
Copy link
Member

@BridgeARBridgeAR left a comment

Choose a reason for hiding this comment

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

Great work! LGTM

lib/assert.js Outdated
functionparseCode(code,offset){
// Lazy load acorn.
if(parseExpressionAt===undefined){
if(parseExpressionAt===undefined||tokenizer===undefined){
Copy link
Member

@BridgeARBridgeARFeb 22, 2023

Choose a reason for hiding this comment

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

Suggested change
if(parseExpressionAt===undefined||tokenizer===undefined){
if(parseExpressionAt===undefined){

The second condition is not needed due to the tokenizer and parseExpressionAt being defined at the same moment.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@BridgeAR
Thanks for your feedback.
I see. I will revert this conditional expression back to the previous one.

lib/assert.js Outdated
// expression is bigger than the provided buffer.
// eslint-disable-next-line no-throw-literal
thrownull;
if(node&&node.node.end>=offset){
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if(node&&node.node.end>=offset){
if(node?.node.end>=offset){

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@BridgeAR
Got it. I will change this code.

@BridgeARBridgeAR added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2023
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2023
@nodejs-github-bot
Copy link
Collaborator

@cola119cola119 added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 24, 2023
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 24, 2023
@nodejs-github-bot
Copy link
Collaborator

@cola119cola119 requested a review from aduh95February 24, 2023 06:55
@BridgeARBridgeAR added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Feb 25, 2023
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 25, 2023
@nodejs-github-bot
Copy link
Collaborator

@aduh95aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Feb 26, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 26, 2023
@nodejs-github-botnodejs-github-bot merged commit 3c0131a into nodejs:mainFeb 26, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 3c0131a

targos pushed a commit that referenced this pull request Mar 13, 2023
Fixes: #30872 PR-URL: #46760 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Kohei Ueno <[email protected]>
targos pushed a commit that referenced this pull request Mar 14, 2023
Fixes: #30872 PR-URL: #46760 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Kohei Ueno <[email protected]>
@targostargos mentioned this pull request Mar 14, 2023
danielleadams pushed a commit that referenced this pull request Apr 11, 2023
Fixes: #30872 PR-URL: #46760 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Kohei Ueno <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

assertIssues and PRs related to the assert subsystem.author readyPRs that have at least one approval, no pending requests for changes, and a CI started.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exception message for assert(0) depends on whitespace

5 participants

@hidecology@nodejs-github-bot@BridgeAR@aduh95@cola119