Skip to content

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Jan 16, 2025

Motivation:

The reflection service is a single type called ReflectionService. The
Health service is a single type (called Health) holding an instance of
the HealthService and a type to provide values to the service. This
indirection isn't necessary, the health service can just hold it
instead. This makes the API more like the reflection service and removes
indirection.

Modifications:

  • Remove indirection from health service and rename from 'Health' to
    'HealthService'.
  • Make the reflection service a struct (it was a class for no good
    reason).

Result:

More consistent API

Motivation:

The reflection service is a single type called `ReflectionService`. The
Health service is a single type (called `Health`) holding an instance of
the `HealthService` and a type to provide values to the service. This
indirection isn't necessary, the health service can just hold it
instead. This makes the API more like the reflection service and removes
indirection.

Modifications:

- Remove indirection from health service and rename from 'Health' to
  'HealthService'.
- Make the reflection service a struct (it was a class for no good
  reason).

Result:

More consistent API
@glbrntt
Copy link
Collaborator Author

glbrntt commented Jan 16, 2025

Note this will be easier to review as two separate commits:

  • 3b35905 contains the actual changes
  • 8b8ad22 renames a couple of files

Both commits together make for a harder diff to review.

@glbrntt glbrntt added the ⚠️ semver/major Breaks existing public API. label Jan 16, 2025
@glbrntt glbrntt marked this pull request as ready for review January 16, 2025 08:07
@glbrntt glbrntt requested a review from gjcairo January 16, 2025 08:08
@glbrntt glbrntt merged commit 0046afa into grpc:main Jan 17, 2025
20 of 22 checks passed
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