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

Add rules to generate a thrift-gen release. #135

Merged
merged 2 commits into from
Dec 30, 2015
Merged

Conversation

thanodnl
Copy link
Contributor

To test generated code on travis we need a version of thrift-gen available during ci. Before we used go get github.com/uber/tchannel-go/thrift/thrift-gen to install a version there. But since go get is unaware of the versions of dependencies used by thrift-gen this unexpectedly broke our tests.

With these changes we can reliably generate a release tarball via make release_thrift_gen and upload it to github to create a release of thrift-gen. We can then download a specific working binary of thrift-gen during ci to test our code generation.

Example usage:

For testing purposes I created an alpha release on https://github.com/thanodnl/tchannel-go/releases/tag/v0.1-alpha to test this on travis.

tar -czf thrift-gen.tar.gz $(RELEASE)
mv thrift-gen.tar.gz $(RELEASE)/


.PHONY: all help clean fmt format get_thrift install install_ci test test_ci vet
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add release_thrift_gen to the .PHONY section

@prashantv
Copy link
Contributor

Cool, some minor comments, otherwise lgtm.

@thanodnl
Copy link
Contributor Author

Thanks for the comments.

I changed the naming of the directories in the tarball a bit so they are in line with the release of thrift https://github.com/prashantv/thrift/releases. This makes the mv at the end of the get-thrift-gen.sh script obsolete. And with that I did some renaming of the internal variables used. Now it is very evident from reading the variables what they are used for.

@prashantv
Copy link
Contributor

lgtm

@thanodnl
Copy link
Contributor Author

Rebased on master for .PHONY

thanodnl added a commit that referenced this pull request Dec 30, 2015
Add rules to generate a thrift-gen release.
@thanodnl thanodnl merged commit a2deac0 into master Dec 30, 2015
@thanodnl thanodnl deleted the release-thrift-gen branch December 30, 2015 16:24
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