Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

tools: enable no-return-await lint rule #17265

Closed
wants to merge 2 commits into from
Closed

tools: enable no-return-await lint rule #17265

wants to merge 2 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Nov 23, 2017

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

tools doc test

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Nov 23, 2017
Florian-R
Florian-R reviewed Nov 23, 2017
View reviewed changes
doc/api/util.md Outdated
@@ -31,7 +31,7 @@ For example:
const util = require('util');

async function fn() {
return await Promise.resolve('hello world');
return Promise.resolve('hello world');
Copy link

Choose a reason for hiding this comment

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

Quoting https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function

(...) because the return value of an async function is implicitly wrapped in Promise.resolve.

So I assume you can drop the Promise.resolve here and in below occurences

Copy link
Member Author

Choose a reason for hiding this comment

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

@Florian-R Done. Thanks for the suggestion!

@Trott
doc,test: remove unnecessary await with return instances
58f7f40 
Remove unnecessary `await` in combination with `return` in preparation
for enabling lint rule.
@Trott
tools: enable no-return-await lint rule
6ff7320 
Enable lint rule that prohibits unnecessary use of `await` with
`return`.
@Trott
Copy link
Member Author

Trott commented Nov 29, 2017

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

@Trott
Copy link
Member Author

Trott commented Dec 1, 2017

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

@Trott
Copy link
Member Author

Trott commented Dec 1, 2017

Landed in 929f343 and 3327ce0.

@Trott Trott closed this Dec 1, 2017
Trott added a commit to Trott/io.js that referenced this pull request Dec 1, 2017
@Trott
doc,test: remove unnecessary await with return instances
929f343 
Remove unnecessary `await` in combination with `return` in preparation
for enabling lint rule.

PR-URL: nodejs#17265
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Trott added a commit to Trott/io.js that referenced this pull request Dec 1, 2017
@Trott
tools: enable no-return-await lint rule
3327ce0 
Enable lint rule that prohibits unnecessary use of `await` with
`return`.

PR-URL: nodejs#17265
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@Trott @MylesBorins
doc,test: remove unnecessary await with return instances
967f972 
Remove unnecessary `await` in combination with `return` in preparation
for enabling lint rule.

PR-URL: #17265
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@Trott @MylesBorins
tools: enable no-return-await lint rule
685b199 
Enable lint rule that prohibits unnecessary use of `await` with
`return`.

PR-URL: #17265
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@Trott @MylesBorins
doc,test: remove unnecessary await with return instances
a256482 
Remove unnecessary `await` in combination with `return` in preparation
for enabling lint rule.

PR-URL: #17265
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@Trott @MylesBorins
tools: enable no-return-await lint rule
6c65e04 
Enable lint rule that prohibits unnecessary use of `await` with
`return`.

PR-URL: #17265
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
Merged
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
@Trott @gibfahn
doc,test: remove unnecessary await with return instances
2af1bf5 
Remove unnecessary `await` in combination with `return` in preparation
for enabling lint rule.

PR-URL: #17265
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
@Trott @gibfahn
tools: enable no-return-await lint rule
053eba0 
Enable lint rule that prohibits unnecessary use of `await` with
`return`.

PR-URL: #17265
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@gibfahn
Copy link
Member

gibfahn commented Dec 19, 2017

@Trott would you mind backporting to 6.x?

@Trott
Copy link
Member Author

Trott commented Dec 19, 2017

@Trott would you mind backporting to 6.x?

According to http://node.green/#ES2017-features-async-functions-await, await did not appear until 7.x, so I think we can skip backporting this to 6.x. Do correct me if I'm wrong!

@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Closed
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@Trott @gibfahn
doc,test: remove unnecessary await with return instances
f1de0da 
Remove unnecessary `await` in combination with `return` in preparation
for enabling lint rule.

PR-URL: #17265
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@Trott @gibfahn
tools: enable no-return-await lint rule
36c977e 
Enable lint rule that prohibits unnecessary use of `await` with
`return`.

PR-URL: #17265
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Merged
@Trott Trott deleted the no-return-await branch January 13, 2022 22:47
@sapphi-red sapphi-red mentioned this pull request Oct 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Florian-R Florian-R Florian-R left review comments

@gibfahn gibfahn gibfahn approved these changes

@vsemozhetbyt vsemozhetbyt vsemozhetbyt approved these changes

@cjihrig cjihrig cjihrig approved these changes

@targos targos targos approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

@lpinca lpinca lpinca approved these changes

@apapirovski apapirovski apapirovski approved these changes

Assignees
No one assigned
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@Trott @gibfahn @apapirovski @Florian-R @lpinca @TimothyGu @targos @cjihrig @vsemozhetbyt @nodejs-github-bot