Skip to content

lib: removes unnecessary else statement - #32662

Closed
JesuHrz wants to merge 1 commit into
nodejs:masterfrom
JesuHrz:jesuhrz/removes-unnecesary-else-block
Closed

lib: removes unnecessary else statement#32662
JesuHrz wants to merge 1 commit into
nodejs:masterfrom
JesuHrz:jesuhrz/removes-unnecesary-else-block

Conversation

@JesuHrz

@JesuHrzJesuHrz commented Apr 4, 2020

Copy link
Copy Markdown
Contributor

This PR removes the else statement inside the readFileHandle function since exists a return in the if statement which makes that the else statement unnecessary

  • 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-botnodejs-github-bot added the fs Issues and PRs related to the fs subsystem / file system. label Apr 4, 2020
@lpincalpinca mentioned this pull request Apr 5, 2020
Closed
2 tasks
jasnell
jasnell reviewed Apr 7, 2020
View reviewed changes
Comment threadlib/internal/fs/promises.js Outdated

@jasnelljasnellApr 7, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This can be further simplified to:

returnoptions.encoding ? result.toString(options.encoding) : result;

@JesuHrzJesuHrzApr 7, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Thanks @jasnell for the feedback, I will make this change right now.

@JesuHrz
JesuHrzforce-pushed the jesuhrz/removes-unnecesary-else-block branch from 1684504 to 36b9d10CompareApril 7, 2020 00:30
juanarbol
juanarbol approved these changes Apr 18, 2020
View reviewed changes

@juanarboljuanarbol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice

@nodejs-github-bot

nodejs-github-bot commented Apr 19, 2020

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Apr 19, 2020

Copy link
Copy Markdown
Collaborator

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

@lpinca

lpinca commented Apr 19, 2020

Copy link
Copy Markdown
Member

Landed in a6a15fe.

lpinca pushed a commit that referenced this pull request Apr 19, 2020
@JesuHrz@lpinca
fs: remove unnecessary else statement
a6a15fe
PR-URL: #32662
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
@lpincalpinca closed this Apr 19, 2020
BethGriggs pushed a commit that referenced this pull request Apr 27, 2020
@JesuHrz@BethGriggs
fs: remove unnecessary else statement
03d02d7
PR-URL: #32662
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
@BethGriggsBethGriggs mentioned this pull request Apr 27, 2020
Merged
BridgeAR pushed a commit that referenced this pull request Apr 28, 2020
@JesuHrz@BridgeAR
fs: remove unnecessary else statement
0b2cff2
PR-URL: #32662
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
@BridgeARBridgeAR mentioned this pull request Apr 28, 2020
Merged
targos pushed a commit that referenced this pull request Apr 30, 2020
@JesuHrz@targos
fs: remove unnecessary else statement
9c18838
PR-URL: #32662
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
@targostargos mentioned this pull request May 2, 2020
Merged
lpinca added a commit to lpinca/node that referenced this pull request May 15, 2020
@lpinca
tools: enable no-else-return lint rule
af70a1c
Refs: nodejs#32644
Refs: nodejs#32662
lpinca added a commit that referenced this pull request May 16, 2020
@lpinca
tools: enable no-else-return lint rule
b533fb3
Refs: #32644
Refs: #32662
PR-URL: #32667
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
codebytere pushed a commit that referenced this pull request May 16, 2020
@lpinca@codebytere
tools: enable no-else-return lint rule
9119344
Refs: #32644
Refs: #32662
PR-URL: #32667
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@lpincalpinca mentioned this pull request Jul 9, 2020
Closed
4 tasks
lpinca added a commit to lpinca/node that referenced this pull request Jul 9, 2020
@lpinca
tools: enable no-else-return lint rule
441e6d9
Refs: nodejs#32644
Refs: nodejs#32662
PR-URL: nodejs#32667
Backport-PR-URL: nodejs#34275
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
codebytere pushed a commit that referenced this pull request Jul 13, 2020
@lpinca@codebytere
tools: enable no-else-return lint rule
a131c72
Refs: #32644
Refs: #32662
PR-URL: #32667
Backport-PR-URL: #34275
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
codebytere pushed a commit that referenced this pull request Jul 14, 2020
@lpinca@codebytere
tools: enable no-else-return lint rule
248f9af
Refs: #32644
Refs: #32662
PR-URL: #32667
Backport-PR-URL: #34275
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes
@lpincalpincalpinca approved these changes
@BridgeARBridgeARBridgeAR approved these changes
@juanarboljuanarboljuanarbol approved these changes
+1 more reviewer
@rickyesrickyesrickyes approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

fsIssues and PRs related to the fs subsystem / file system.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@JesuHrz@nodejs-github-bot@lpinca@jasnell@BridgeAR@juanarbol@rickyes