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/openapi): simplify openapi translator #80

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/openapi

@julieqiu julieqiu changed the title impl: simplify openapi translator impl(generator/internal/genclient/translator/openapi): simplify openapi translator Nov 4, 2024
@julieqiu julieqiu changed the title impl(generator/internal/genclient/translator/openapi): simplify openapi translator impl(internal/genclient/translator/openapi): simplify openapi 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.

This will complicate some pending PRs. Maybe do all the other cleanups and post-pone removing the translator until more of the structure is filled up? Other ideas?

}

func (t *Translator) makeMessageFields(messageName string, message *base.Schema) ([]*genclient.Field, error) {
func makeMessageFields(messageName string, message *base.Schema) ([]*genclient.Field, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Soon we will need the t *Translator object again:

https://github.com/googleapis/google-cloud-rust/pull/71/files#diff-51632d73d29bc895749a0c2b04811475cddc5dd97a1f13b971f6509daf2dab85R304

We need to dynamically insert some messages into t.state.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will comment on that PR

Copy link
Contributor

Choose a reason for hiding this comment

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

You need to rebase, there is a conflict after I merged #71.

@julieqiu julieqiu merged commit 7333d58 into main Nov 4, 2024
15 checks passed
@julieqiu julieqiu deleted the julie-pr7 branch November 4, 2024 16:55
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