Skip to content

build: set @returns description for void async methods automatically #17751

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
Merged
merged 1 commit into from
Nov 20, 2019
Merged

build: set @returns description for void async methods automatically #17751

merged 1 commit into from
Nov 20, 2019

Conversation

devversion
Copy link
Member

Automatically sets the @returns tag description for asynchronous
methods which do not return any value.

This is a proposal based on the commit in:
#17723 (comment)

This processor added a @returns to, so it seems like it can be useful. Though I'm afraid we need a more generic return description? i.e. is it always an action? @jelbourn

Adding to cdk/testing/LocatorFactory.forceStabilize_0()
Adding to cdk/testing/LocatorFactory.waitForTasksOutsideAngular_0()
Adding to cdk/testing/ComponentHarness.forceStabilize()
Adding to cdk/testing/ComponentHarness.waitForTasksOutsideAngular()
Adding to cdk/testing/HarnessEnvironment.forceStabilize_0()
Adding to cdk/testing/HarnessEnvironment.waitForTasksOutsideAngular_0()
Adding to cdk/testing/TestElement.blur_0()
Adding to cdk/testing/TestElement.clear_0()
Adding to cdk/testing/TestElement.click_0()
Adding to cdk/testing/TestElement.focus_0()
Adding to cdk/testing/TestElement.hover_0()
Adding to cdk/testing/TestElement.sendKeys_0()
Adding to cdk/testing/TestElement.sendKeys_1()
Adding to cdk/testing/protractor/ProtractorElement.blur()
Adding to cdk/testing/protractor/ProtractorElement.clear()
Adding to cdk/testing/protractor/ProtractorElement.click()
Adding to cdk/testing/protractor/ProtractorElement.focus()
Adding to cdk/testing/protractor/ProtractorElement.hover()
Adding to cdk/testing/protractor/ProtractorElement.sendKeys()
Adding to cdk/testing/protractor/ProtractorElement.sendKeys_0()
Adding to cdk/testing/protractor/ProtractorElement.sendKeys_1()
Adding to cdk/testing/protractor/ProtractorHarnessEnvironment.forceStabilize()
Adding to cdk/testing/protractor/ProtractorHarnessEnvironment.waitForTasksOutsideAngular()
Adding to cdk/testing/testbed/TestbedHarnessEnvironment.forceStabilize()
Adding to cdk/testing/testbed/TestbedHarnessEnvironment.waitForTasksOutsideAngular()
Adding to cdk/testing/testbed/UnitTestElement.blur()
Adding to cdk/testing/testbed/UnitTestElement.clear()
Adding to cdk/testing/testbed/UnitTestElement.click()
Adding to cdk/testing/testbed/UnitTestElement.focus()
Adding to cdk/testing/testbed/UnitTestElement.hover()
Adding to cdk/testing/testbed/UnitTestElement.sendKeys()
Adding to cdk/testing/testbed/UnitTestElement.sendKeys_0()
Adding to cdk/testing/testbed/UnitTestElement.sendKeys_1()
Adding to material/autocomplete/testing/MatAutocompleteHarness.focus()
Adding to material/autocomplete/testing/MatAutocompleteHarness.blur()
Adding to material/autocomplete/testing/MatAutocompleteHarness.enterText()
Adding to material/autocomplete/testing/MatAutocompleteHarness.selectOption()
Adding to material/button/testing/MatButtonHarness.click()
Adding to material/button/testing/MatButtonHarness.focus()
Adding to material/button/testing/MatButtonHarness.blur()
Adding to material/checkbox/testing/MatCheckboxHarness.focus()
Adding to material/checkbox/testing/MatCheckboxHarness.blur()
Adding to material/checkbox/testing/MatCheckboxHarness.toggle()
Adding to material/checkbox/testing/MatCheckboxHarness.check()
Adding to material/checkbox/testing/MatCheckboxHarness.uncheck()
Adding to material/dialog/testing/MatDialogHarness.close()
Adding to material/menu/testing/MatMenuHarness.focus()
Adding to material/menu/testing/MatMenuHarness.blur()
Adding to material/menu/testing/MatMenuHarness.open()
Adding to material/menu/testing/MatMenuHarness.close()
Adding to material/menu/testing/MatMenuHarness.clickItem()
Adding to material/menu/testing/MatMenuItemHarness.focus()
Adding to material/menu/testing/MatMenuItemHarness.blur()
Adding to material/menu/testing/MatMenuItemHarness.click()
Adding to material/radio/testing/MatRadioGroupHarness.checkRadioButton()
Adding to material/radio/testing/MatRadioButtonHarness.focus()
Adding to material/radio/testing/MatRadioButtonHarness.blur()
Adding to material/radio/testing/MatRadioButtonHarness.check()
Adding to material/slide-toggle/testing/MatSlideToggleHarness.focus()
Adding to material/slide-toggle/testing/MatSlideToggleHarness.blur()
Adding to material/slide-toggle/testing/MatSlideToggleHarness.toggle()
Adding to material/slide-toggle/testing/MatSlideToggleHarness.check()
Adding to material/slide-toggle/testing/MatSlideToggleHarness.uncheck()
Adding to material/slider/testing/MatSliderHarness.setValue()
Adding to material/slider/testing/MatSliderHarness.focus()
Adding to material/slider/testing/MatSliderHarness.blur()
Adding to material/snack-bar/testing/MatSnackBarHarness.dismissWithAction()
Adding to material/tabs/testing/MatTabGroupHarness.selectTab()
Adding to material/tabs/testing/MatTabHarness.select()

@devversion devversion added needs: discussion Further discussion with the team is needed before proceeding pr: merge safe target: patch This PR is targeted for the next patch release labels Nov 19, 2019
@devversion devversion requested a review from a team as a code owner November 19, 2019 11:56
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 19, 2019
@devversion devversion mentioned this pull request Nov 19, 2019
Merged
@devversion devversion force-pushed the build/docs-auto-set-async-returns-description branch from 85ca004 to 80b9745 Compare November 19, 2019 14:56
jelbourn
jelbourn reviewed Nov 19, 2019
View reviewed changes
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

I like it; I think "action" is a broad enough term to work for everything

tools/dgeni/processors/async-return-description.ts Outdated
}
const typeString = getTypeOfFunctionLikeDoc(doc);
if (!doc.returns && typeString === 'Promise<void>') {
doc.returns = {description: 'Promise that resolves when the action is complete.'};
Copy link
Member

Choose a reason for hiding this comment

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

Let's do

Promise that resolves when the action completes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that sounds better.

@devversion
build: set @returns description for void async methods automatically
6093089 
Automatically sets the `@returns` tag description for asynchronous
methods which do not return any value.

This is a proposal based on the commit in:
angular#17723 (comment)
@devversion devversion removed the needs: discussion Further discussion with the team is needed before proceeding label Nov 19, 2019
@devversion devversion force-pushed the build/docs-auto-set-async-returns-description branch from 80b9745 to 6093089 Compare November 19, 2019 17:01
jelbourn
jelbourn approved these changes Nov 20, 2019
View reviewed changes
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Nov 20, 2019
@jelbourn jelbourn merged commit 1512721 into angular:master Nov 20, 2019
mmalerba added a commit to mmalerba/components that referenced this pull request Nov 20, 2019
@mmalerba
remove explicit @return for async void fns (covered by angular#17751)
f503ed0
mmalerba added a commit to mmalerba/components that referenced this pull request Nov 21, 2019
@mmalerba
remove explicit @return for async void fns (covered by angular#17751)
d6addeb
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jelbourn jelbourn jelbourn approved these changes

Assignees

@jelbourn jelbourn

Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@devversion @jelbourn @googlebot