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

impl(internal/genclient/translator/protobuf): simplify protobuf translator #81

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

julieqiu
Copy link
Member

@julieqiu julieqiu commented Nov 4, 2024

The Translator struct isn't necessary, so it is removed to simplify the code in generator/internal/genclient/translator/protobuf

@julieqiu julieqiu changed the title impl: simplify protobuf translator impl(generator/internal/genclient/translator/protobuf): simplify protobuf translator Nov 4, 2024
@julieqiu julieqiu changed the title impl(generator/internal/genclient/translator/protobuf): simplify protobuf translator impl(internal/genclient/translator/protobuf): simplify protobuf translator Nov 4, 2024
Copy link
Contributor

@coryan coryan left a comment

Choose a reason for hiding this comment

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

Almost all the cleanups look great to me.

The removal of the Translator object is maybe six of one half a doze of the other... we can always change it back.

message.Name = m.GetName()
message.ID = mFQN
message.Parent = parent
func processMessage(state *genclient.APIState, m *descriptorpb.DescriptorProto, mFQN string, parent *genclient.Message) *genclient.Message {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't claim to know what is idiomatic in Go, but this is basically making the functions methods on the state parameter...

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to do away with the Translator struct, since it didn't seem to be adding anything and removing made things easier to reason about

@julieqiu julieqiu merged commit ac092a3 into main Nov 4, 2024
15 checks passed
@julieqiu julieqiu deleted the julie-pr8 branch November 4, 2024 14:00
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.

2 participants