Skip to content

Cleanup of grpc go generation code#136

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

Cleanup of grpc go generation code#136
enocom merged 1 commit intoagones-dev:masterfrom
markmandel:cleanup/go-sdk

Conversation

@markmandel
Copy link
Collaborator

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
Collaborator Author

@enocom Curious if you see any issues here?

@enocom
Copy link
Contributor

enocom commented Mar 9, 2018

@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
Collaborator 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
Collaborator Author

Also makes it go-gettable

@agones-bot
Copy link
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
Copy link
Contributor

enocom commented Mar 9, 2018

SGTM. Duplicate unnecessary, for sure.

Copy link
Contributor

@enocom enocom left a comment

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