Skip to content

Comments

services: set the default serving state of all services to SERVING#5274

Merged
carl-mastrangelo merged 1 commit intogrpc:masterfrom
carl-mastrangelo:hsm
Jan 24, 2019
Merged

services: set the default serving state of all services to SERVING#5274
carl-mastrangelo merged 1 commit intogrpc:masterfrom
carl-mastrangelo:hsm

Conversation

@carl-mastrangelo
Copy link
Contributor

No description provided.

Copy link
Member

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

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

Should there be a documentation update?

@carl-mastrangelo
Copy link
Contributor Author

Filed grpc/grpc#17805

@markdroth
Copy link
Member

This seems incorrect to me. We should not set any service other than the empty string to SERVING by default. None of the other languages do that.

@ejona86
Copy link
Member

ejona86 commented Jan 24, 2019

@markdroth, SERVICE_NAME_ALL_SERVICES = ""

@carl-mastrangelo, I was meaning in the Javadoc.

@markdroth
Copy link
Member

Ah, okay. Carry on, then. :)

@ejona86 ejona86 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Jan 24, 2019
@grpc-kokoro grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Jan 24, 2019
@carl-mastrangelo
Copy link
Contributor Author

Added a comment.

* {@link #getHealthService()} method.
* The health status manager can update the health statuses of the server.
*
* <p>
Copy link
Member

Choose a reason for hiding this comment

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

What's with this formatting? Don't intent because of the <p>. Don't include the </p>. End in a period.

* <p>The default, empty-string, service name, {@link #SERVICE_NAME_ALL_SERVICES}, is initialized to
* {@link ServingStatus#SERVING}.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's what IntelliJ likes. Seems arbitrary.

* The health status manager can update the health statuses of the server.
*
* <p>The default, empty-string, service name, {@link #SERVICE_NAME_ALL_SERVICES}, is initialized to
* {@link ServingStatus#SERVING}.
Copy link
Member

Choose a reason for hiding this comment

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

Remove the extra indentation here as well.

It's part of the style guide. https://google.github.io/styleguide/javaguide.html#s7.1.2-javadoc-paragraphs

@carl-mastrangelo carl-mastrangelo added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Jan 24, 2019
@grpc-kokoro grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Jan 24, 2019
@carl-mastrangelo carl-mastrangelo merged commit 32fc0bc into grpc:master Jan 24, 2019
@carl-mastrangelo carl-mastrangelo deleted the hsm branch January 24, 2019 23:19
@lock lock bot locked as resolved and limited conversation to collaborators Apr 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants