Skip to content

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Nov 21, 2024

Motivation:

MethodDescriptor uses a string to represent the fully qualified name of the service the method belongs to. ServiceDescriptor also represents a fully qualified name of a service. It'd make more sense for MethodDescriptor to use a ServiceDescriptor instead of a string to describe the service.

Modifications:

  • ServiceDescriptor now stores the fully qualified service name (as this is generally more useful) and computes the package and unqualified service name.
  • ServiceDescriptor can now be created from the fully qualified name or from a separate package and service name.
  • MethodDescriptor now holds a ServiceDescriptor instead of a String for the service

Result:

More coherent API

Motivation:

MethodDescriptor uses a string to represent the fully qualified name of
the service the method belongs to. ServiceDescriptor also represents a
fully qualified name of a service. It'd make more sense for
MethodDescriptor to use a ServiceDescriptor instead of a string to
describe the service.

Modifications:

- ServiceDescriptor now stores the fully qualified service name (as this
  is generally more useful) and computes the package and unqualified
  service name.
- ServiceDescriptor can now be created from the fully qualified name or
  from a separate package and service name.
- MethodDescriptor now holds a ServiceDescriptor instead of a String for
  the service

Result:

More coherent API
@glbrntt glbrntt added the ⚠️ semver/major Breaks existing public API. label Nov 21, 2024
@glbrntt
Copy link
Collaborator Author

glbrntt commented Nov 22, 2024

API breakage is expected and okay.

@glbrntt glbrntt merged commit ef48ef8 into grpc:main Nov 22, 2024
42 of 45 checks passed
@glbrntt glbrntt deleted the v2/use-service-desciptor-in-method branch November 22, 2024 09:20
@rnro rnro mentioned this pull request Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ semver/major Breaks existing public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants