Skip to content

assert: remove unneeded condition#11314

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:refactor-assert
Closed

assert: remove unneeded condition#11314
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:refactor-assert

Conversation

@Trott
Copy link
Copy Markdown
Member

@Trott Trott commented Feb 11, 2017

In expectedException(), actual is guaranteed to be truthy. We only need to check expected for truthiness.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

assert
@nodejs-github-bot nodejs-github-bot added the assert Issues and PRs related to the assert subsystem. label Feb 11, 2017
@mscdex
Copy link
Copy Markdown
Contributor

mscdex commented Feb 11, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/6361/
gibfahn
gibfahn reviewed Feb 12, 2017
View reviewed changes
Comment thread lib/assert.js Outdated
Copy link
Copy Markdown
Member

@gibfahn gibfahn Feb 12, 2017

Choose a reason for hiding this comment

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

Nit: full stop at the end of the sentence. I assume actual doesn't need to be capitalised as it's a variable.
gibfahn
gibfahn reviewed Feb 12, 2017
View reviewed changes
Comment thread lib/assert.js Outdated
Copy link
Copy Markdown
Member

@gibfahn gibfahn Feb 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Nit: full stop at the end of the sentence. I assume actual doesn't need to be capitalised as it's a variable.

EDIT: Not sure why this has appeared twice...
@gibfahn
Copy link
Copy Markdown
Member

gibfahn commented Feb 12, 2017

Also I guess we should note that this depends on #11304 (assuming dead code elimination doesn't count as a bugfix).
mhdawson
mhdawson approved these changes Feb 14, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM
@Trott
Copy link
Copy Markdown
Member Author

Trott commented Feb 15, 2017

assert is now Stable instead of Locked so this can land
@Trott
Copy link
Copy Markdown
Member Author

Trott commented Feb 15, 2017

new CI: https://ci.nodejs.org/job/node-test-pull-request/6435/
Trott added a commit to Trott/io.js that referenced this pull request Feb 15, 2017
@Trott
assert: remove unneeded condition
3429991 
PR-URL: nodejs#11314
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@Trott
Copy link
Copy Markdown
Member Author

Trott commented Feb 15, 2017

Landed in 3429991
@Trott Trott closed this Feb 15, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Feb 16, 2017
@Trott
assert: remove unneeded condition
1a33851 
PR-URL: nodejs#11314
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
italoacasas pushed a commit that referenced this pull request Feb 22, 2017
@Trott
assert: remove unneeded condition
6a2f330 
PR-URL: #11314
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@italoacasas italoacasas mentioned this pull request Feb 25, 2017
Merged
jasnell pushed a commit that referenced this pull request Mar 7, 2017
@Trott @jasnell
assert: remove unneeded condition
488cb02 
PR-URL: #11314
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
jasnell pushed a commit that referenced this pull request Mar 7, 2017
@Trott @jasnell
assert: remove unneeded condition
f266573 
PR-URL: #11314
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
@Trott @MylesBorins
assert: remove unneeded condition
32b264c 
PR-URL: #11314
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
@Trott @MylesBorins
assert: remove unneeded condition
090037a 
PR-URL: #11314
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
Merged
@Trott Trott deleted the refactor-assert branch January 13, 2022 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

assert Issues and PRs related to the assert subsystem.

10 participants

@Trott @mscdex @gibfahn @jasnell @thefourtheye @lpinca @targos @cjihrig @mhdawson @nodejs-github-bot