◐ Shell
reader mode source ↗
Skip to content

tools: require function declarations#12711

Closed
Trott wants to merge 2 commits into
nodejs:masterfrom
Trott:func-style
Closed

tools: require function declarations#12711
Trott wants to merge 2 commits into
nodejs:masterfrom
Trott:func-style

Conversation

@Trott

@Trott Trott commented Apr 28, 2017

Copy link
Copy Markdown
Member

Welp, not sure how this will be received, but since it is the predominant style in our code base and since we are occasionally offering nits to new contributors to use the style to avoid potential confusion over the Temporal Dead Zone....

In lib, doc, and test (but mostly test): Replace function expressions with function declarations in preparation for a lint rule requiring function declarations.

Then: Except for arrow functions, require function declarations instead of function expressions via linting. This is the predominant style in our code base (77 instances of expressions to 2344 instances of declarations).

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

tools test lib doc

@Trott Trott added doc Issues and PRs related to the documentations. lib / src Issues and PRs related to general changes in the lib or src directory. test Issues and PRs related to the tests. tools labels Apr 28, 2017
@nodejs-github-bot nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. tls Issues and PRs related to the tls subsystem. tools Issues and PRs related to the tools directory. util Issues and PRs related to the built-in util module. labels Apr 28, 2017
@vsemozhetbyt

vsemozhetbyt commented Apr 28, 2017

Copy link
Copy Markdown
Contributor

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

@Trott Trott force-pushed the func-style branch 2 times, most recently from f79ef5b to fbac2cf Compare April 28, 2017 20:16
Trott added 2 commits April 28, 2017 13:26
@Trott
test,lib,doc: use function declarations …
fe3a22a 
Replace function expressions with function declarations in preparation
for a lint rule requiring function declarations.
@Trott
tools: require function declarations …
791a56f 
Except for arrow functions, require function declarations instead of
function expressions via linting. This is the predominant style in our
code base (77 instances of expressions to 2344 instances of
declarations).
@Trott

Trott commented Apr 28, 2017

Copy link
Copy Markdown
Member Author

Arrow function and quotation mark nits applied.

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

aqrln added a commit to aqrln/node that referenced this pull request Apr 28, 2017
@aqrln
test: make common.noop a getter …
c94f98a 
Make `common.noop` a getter that returns a new function object each time
the propery is read, not the same one. The old behavior could possibly
lead to subtle and hard to catch bugs in some cases.

Refs: nodejs#12711 (comment)
@aqrln aqrln mentioned this pull request Apr 28, 2017
Closed
4 tasks
@refack refack mentioned this pull request Apr 30, 2017
Closed
3 tasks
19 hidden items Load more…
Trott added a commit to Trott/io.js that referenced this pull request May 1, 2017
@Trott
test,lib,doc: use function declarations …
a180259 
Replace function expressions with function declarations in preparation
for a lint rule requiring function declarations.

PR-URL: nodejs#12711
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Trott added a commit to Trott/io.js that referenced this pull request May 1, 2017
@Trott
tools: require function declarations …
aea7269 
Except for arrow functions, require function declarations instead of
function expressions via linting. This is the predominant style in our
code base (77 instances of expressions to 2344 instances of
declarations).

PR-URL: nodejs#12711
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@Trott

Trott commented May 1, 2017

Copy link
Copy Markdown
Member Author

Landed in a180259 and aea7269

@Trott Trott closed this May 1, 2017
anchnk pushed a commit to anchnk/node that referenced this pull request May 6, 2017
@Trott
tools: require function declarations …
2d1bf1f 
Except for arrow functions, require function declarations instead of
function expressions via linting. This is the predominant style in our
code base (77 instances of expressions to 2344 instances of
declarations).

PR-URL: nodejs#12711
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@jasnell jasnell mentioned this pull request May 11, 2017
Closed
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
@gibfahn

gibfahn commented Jun 18, 2017

Copy link
Copy Markdown
Member

@Trott would you be willing to backport this to v6.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

Trott added a commit to Trott/io.js that referenced this pull request Jun 19, 2017
@Trott
test,lib,doc: use function declarations …
c4c7e9b 
Replace function expressions with function declarations in preparation
for a lint rule requiring function declarations.

PR-URL: nodejs#12711
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Trott added a commit to Trott/io.js that referenced this pull request Jun 19, 2017
@Trott
tools: require function declarations …
35cc20b 
Except for arrow functions, require function declarations instead of
function expressions via linting. This is the predominant style in our
code base (77 instances of expressions to 2344 instances of
declarations).

PR-URL: nodejs#12711
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@Trott Trott mentioned this pull request Jun 19, 2017
Closed
@Trott

Trott commented Jun 19, 2017

Copy link
Copy Markdown
Member Author

@gibfahn Backported in #13774

gibfahn pushed a commit that referenced this pull request Jun 20, 2017
@Trott @gibfahn
test,lib,doc: use function declarations …
95731fe 
Replace function expressions with function declarations in preparation
for a lint rule requiring function declarations.

PR-URL: #12711
Backport-PR-URL: #13774
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
gibfahn pushed a commit that referenced this pull request Jun 20, 2017
@Trott @gibfahn
tools: require function declarations …
b58972a 
Except for arrow functions, require function declarations instead of
function expressions via linting. This is the predominant style in our
code base (77 instances of expressions to 2344 instances of
declarations).

PR-URL: #12711
Backport-PR-URL: #13774
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2017
@Trott @MylesBorins
test,lib,doc: use function declarations …
7e6a956 
Replace function expressions with function declarations in preparation
for a lint rule requiring function declarations.

PR-URL: #12711
Backport-PR-URL: #13774
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2017
@Trott @MylesBorins
tools: require function declarations …
3d564a4 
Except for arrow functions, require function declarations instead of
function expressions via linting. This is the predominant style in our
code base (77 instances of expressions to 2344 instances of
declarations).

PR-URL: #12711
Backport-PR-URL: #13774
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 18, 2017
Merged
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Apr 10, 2018
@Trott
test,lib,doc: use function declarations …
b91ac10 
Replace function expressions with function declarations in preparation
for a lint rule requiring function declarations.

PR-URL: nodejs#12711
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 16, 2018
@Trott @MylesBorins
test,lib,doc: use function declarations …
b2bf6c8 
Replace function expressions with function declarations in preparation
for a lint rule requiring function declarations.

Backport-PR-URL: #19447
PR-URL: #12711
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Apr 16, 2018
Merged
@Trott Trott deleted the func-style branch January 13, 2022 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell left review comments

+5 more reviewers

@mscdex mscdex mscdex left review comments

@hiroppy hiroppy hiroppy left review comments

@aqrln aqrln aqrln left review comments

@vsemozhetbyt vsemozhetbyt vsemozhetbyt approved these changes

@gibfahn gibfahn gibfahn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. lib / src Issues and PRs related to general changes in the lib or src directory. test Issues and PRs related to the tests. tls Issues and PRs related to the tls subsystem. tools Issues and PRs related to the tools directory. util Issues and PRs related to the built-in util module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@Trott @vsemozhetbyt @gibfahn @mscdex @jasnell @hiroppy @aqrln @nodejs-github-bot