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.

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

Simplify typealias translator #2126

Merged
merged 1 commit into from
Nov 20, 2024
Merged

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Nov 20, 2024

Motivation:

The typealias used to generte an enum per package name and nested enums for each service within that namespace. In order for that to work, the services had to be grouped by namespace. This is no longer the case, each service is generated within a combined package-service enum.

It's simpler to just generate the services in the order they are created.

Modifications:

  • Remove the sorting
  • Remove tests which test the unwanted behaviour

Result:

Less code

Motivation:

The typealias used to generte an enum per package name and nested enums
for each service within that namespace. In order for that to work, the
services had to be grouped by namespace. This is no longer the case,
each service is generated within a combined package-service enum.

It's simpler to just generate the services in the order they are
created.

Modifications:

- Remove the sorting
- Remove tests which test the unwanted behaviour

Result:

Less code
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Nov 20, 2024
@glbrntt glbrntt enabled auto-merge (squash) November 20, 2024 15:41
@glbrntt glbrntt requested a review from rnro November 20, 2024 15:41
Copy link
Collaborator

@rnro rnro left a comment

Choose a reason for hiding this comment

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

LGTM

@glbrntt glbrntt merged commit 5260579 into grpc:main Nov 20, 2024
44 of 46 checks passed
@glbrntt glbrntt deleted the v2/remove-unnecessary-tests branch November 20, 2024 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants