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

Rename service.Application.SignalTestComplete to Shutdown #2260

Closed
rakyll opened this issue Dec 7, 2020 · 4 comments · Fixed by #2277
Closed

Rename service.Application.SignalTestComplete to Shutdown #2260

rakyll opened this issue Dec 7, 2020 · 4 comments · Fixed by #2277
Labels
area:service bug Something isn't working priority:p1 High release:required-for-ga Must be resolved before GA release

Comments

@rakyll
Copy link
Contributor

rakyll commented Dec 7, 2020

SignalTestComplete doesn't belong to an external API, rename this method to Shutdown and follow up with #483 in the future.
@rakyll rakyll added the bug Something isn't working label Dec 7, 2020
@rakyll
Copy link
Contributor Author

rakyll commented Dec 7, 2020

Alternative, document that SignalTestComplete API is not stable and might be removed in the future.

@andrewhsu andrewhsu added area:service priority:p1 High release:required-for-ga Must be resolved before GA release labels Dec 9, 2020
@tigrannajaryan
Copy link
Member

+1. We should rename to Shutdown and also rename stopTestChan to just stopChan. I can't see anything special about this stopping that is only applicable to tests. It is just a way to stop the service.

@tigrannajaryan tigrannajaryan removed their assignment Dec 10, 2020
@tigrannajaryan
Copy link
Member

@rakyll would you like to submit a PR that fixes this?

@rakyll
Copy link
Contributor Author

rakyll commented Dec 11, 2020

Surely.

@rakyll rakyll mentioned this issue Dec 11, 2020
Merged
tigrannajaryan pushed a commit that referenced this issue Dec 11, 2020
@rakyll
Rename SignalTestComplete to Shutdown (#2277)
4d60560 
SignalTestComplete shouldn't be in the public API and Application should provide a Shutdown method instead. This change is fixing the public API, behavior of Shutdown will be fixed in the future.

Fixes #2260.
@eddyleelin eddyleelin mentioned this issue May 16, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area:service bug Something isn't working priority:p1 High release:required-for-ga Must be resolved before GA release
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Rename SignalTestComplete to Shutdown
3 participants
@andrewhsu @rakyll @tigrannajaryan