Skip to content

Cleanup of grpc go generation code - #136

Merged
enocom merged 1 commit into
agones-dev:masterfrom
markmandel:cleanup/go-sdk
Mar 9, 2018
Merged

enocom merged 1 commit into
agones-dev:masterfrom
markmandel:cleanup/go-sdk

Conversation

@markmandel

Copy link
Copy Markdown
Member

I'd previously generated this in two places, and looking at it through Godoc, it seemed like a code smell.

Having the generated code in a single place seems like the cleaner option.

I'd previously generated this in two places, and looking
at it through Godoc, it seemed like a code smell.

Having the generated code in a single place seems like the cleaner
option.
@markmandel markmandel added the kind/cleanup Refactoring code, fixing up documentation, etc label Mar 9, 2018
@markmandel markmandel added this to the 0.2 milestone Mar 9, 2018
@markmandel

Copy link
Copy Markdown
Member Author

@enocom Curious if you see any issues here?

@enocom

enocom commented Mar 9, 2018

Copy link
Copy Markdown
Contributor

@markmandel Yeah, definitely don't need two generated files. The C++ generated stuff might even be deleted too.

In theory, the proto file is enough, but since we're providing a Go library, we might as well have the generated code for people consuming things within pkg.

@markmandel
markmandel requested a review from enocom March 9, 2018 23:27
@markmandel

Copy link
Copy Markdown
Member Author

Personally, I prefer to keep generated files in the repo - makes clear what is going on (the CRD client is all generated too) - as long as it's header'd appropriately.

Kinda like vendoring dependencies 😄

@markmandel

Copy link
Copy Markdown
Member Author

Also makes it go-gettable

@agones-bot

Copy link
Copy Markdown
Collaborator

Build Succeeded 👏

Build Id: a2980d51-f540-4758-95e6-64c07f487614

The following development artifacts have been built, and will exist for the next 30 days:

@enocom

enocom commented Mar 9, 2018 •

Copy link
Copy Markdown
Contributor

SGTM. Duplicate unnecessary, for sure.

@enocom enocom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

@enocom
enocom merged commit b47d77f into agones-dev:master Mar 9, 2018
@markmandel
markmandel deleted the cleanup/go-sdk branch March 9, 2018 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/cleanup Refactoring code, fixing up documentation, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants