Skip to content

Conversation

@maggie-lou
Copy link
Collaborator

I realized that in the generated code, it automatically appends Server to the service name. So with the previous name, it was named CacheServiceServer

@luluz66
Copy link
Contributor

luluz66 commented Dec 9, 2025

It seams like we have examples with Service suffix. MetadataService and AuthService. Another option is to name the service Api, since the package name indicates the service name. For example raft_service.Api and ping.Api. I felt API might make sense, so cache_service.Api vs cache_service.Cache

@maggie-lou
Copy link
Collaborator Author

It seams like we have examples with Service suffix. MetadataService and AuthService. Another option is to name the service Api, since the package name indicates the service name. For example raft_service.Api and ping.Api. I felt API might make sense, so cache_service.Api vs cache_service.Cache

With this PR, it will look like cache_service.CacheServer.
Previously, it would look like cache_service.CacheServiceServer

We often abbreviate proto package names, so I don't like naming the service API because it could create ambiguity. Ex. cspb.APIServer

Another option is to move the cache service definition into cache.proto. So it would be cache.CacheServer

Honestly I don't have a strong preference here, but cache_service.CacheServer is still probably my choice

@vanja-p
Copy link
Contributor

vanja-p commented Dec 10, 2025

I like cache_service.CacheServer. I don't really like having API or Service in the name, since it's redundant: service CacheService. The remote execution services are good examples: ActionCache, Execution, ContentAddressableStorage.

Another reason I don't like API is that it irrationally bothers me when I see it written as Api because of camelcase.

@maggie-lou maggie-lou merged commit bab5df7 into master Dec 11, 2025
11 checks passed
@maggie-lou maggie-lou deleted the svc_name branch December 11, 2025 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants