Skip to content

lib: update indentation of ternaries - #14247

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:indent-ternaries-in-lib
Closed

lib: update indentation of ternaries#14247
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:indent-ternaries-in-lib

Conversation

@Trott

Copy link
Copy Markdown
Member

In preparation for stricter indentation linting and to increase code
clarity, update indentation for ternaries in lib.

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

lib tools

@TrottTrott added lib / src Issues and PRs related to general changes in the lib or src directory. tools Issues and PRs related to the tools directory. labels Jul 15, 2017
@nodejs-github-botnodejs-github-bot added events Issues and PRs related to the events subsystem / EventEmitter. net Issues and PRs related to the net subsystem. url Issues and PRs related to the legacy built-in url module. labels Jul 15, 2017
@addaleax

Copy link
Copy Markdown
Member

There’s a typo in the commit message ;)

Comment threadlib/url.js Outdated

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 doesn’t need to be a ternary anyway, right? Just result.host && result.host.indexOf('@') > 0 && result.host.split('@') works as well?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Good idea! I don't think there's much of a chance this is being done as a ternary because it's more performant than the alternative approach you're suggesting here. But that's usually behind my reluctance to alter anything other than whitespace in these things. :-D

@Trott

Copy link
Copy Markdown
MemberAuthor

There’s a typo in the commit message ;)

Fixed. Thanks!

@Trott
Trottforce-pushed the indent-ternaries-in-lib branch from 379141d to 77b868aCompareJuly 15, 2017 04:07
@TrottTrott changed the title lib: update indentaion of ternarieslib: update indentation of ternariesJul 15, 2017
@Trott
Trottforce-pushed the indent-ternaries-in-lib branch from 77b868a to c915e9fCompareJuly 15, 2017 06:21
In preparation for stricter indentation linting and to increase code
clarity, update indentation for ternaries in lib.
@Trott

Copy link
Copy Markdown
MemberAuthor

Trott added a commit to Trott/io.js that referenced this pull request Jul 18, 2017
In preparation for stricter indentation linting and to increase code
clarity, update indentation for ternaries in lib.
PR-URL: nodejs#14247
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott

Copy link
Copy Markdown
MemberAuthor

Landed in d14c132

@TrottTrott closed this Jul 18, 2017
@addaleaxaddaleax mentioned this pull request Jul 18, 2017
addaleax pushed a commit that referenced this pull request Jul 18, 2017
In preparation for stricter indentation linting and to increase code
clarity, update indentation for ternaries in lib.
PR-URL: #14247
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Jul 19, 2017
In preparation for stricter indentation linting and to increase code
clarity, update indentation for ternaries in lib.
PR-URL: #14247
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Trott added a commit to Trott/io.js that referenced this pull request Aug 15, 2017
In preparation for stricter indentation linting and to increase code
clarity, update indentation for ternaries in lib.
PR-URL: nodejs#14247
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
In preparation for stricter indentation linting and to increase code
clarity, update indentation for ternaries in lib.
Backport-PR-URL: #14835
PR-URL: #14247
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request Aug 16, 2017
MylesBorins pushed a commit that referenced this pull request Sep 5, 2017
In preparation for stricter indentation linting and to increase code
clarity, update indentation for ternaries in lib.
Backport-PR-URL: #14835
PR-URL: #14247
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott
Trott deleted the indent-ternaries-in-lib branch January 13, 2022 22:46
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

eventsIssues and PRs related to the events subsystem / EventEmitter.lib / srcIssues and PRs related to general changes in the lib or src directory.netIssues and PRs related to the net subsystem.toolsIssues and PRs related to the tools directory.urlIssues and PRs related to the legacy built-in url module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@Trott@addaleax@jasnell@lpinca@targos@tniessen@joyeecheung@gibfahn@MylesBorins@nodejs-github-bot