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 version_constant and check from generated code #274

Merged
merged 2 commits into from
Sep 15, 2020

Conversation

marioizquierdo
Copy link
Contributor

The PR #264 includes new server options in the twirp package that are used by the generated code. If the protoc-gen-twirp plugin used to generate code uses the latest version, but the twirp package in the project is an older version, the generated code will fail with a compile-time error. Without this constant, the error is something like clientOpts.PathPrefix() function does not exist. With this constant, the error is more descriptive, like twirp.TwirpPackageIsVersion7 does not exist, which better points to the solution.

This pattern is similar to protobuf generated code, that uses compile-time assertions like proto.ProtoPackageIsVersion4 to make sure the right version of the proto library is included in the destination project.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

t.P(`// is compatible with the twirp package used in your project.`)
t.P(`// A compilation error at this line likely means your copy of the`)
t.P(`// twirp package needs to be updated.`)
t.P(`const _ = `, t.pkgs["twirp"], `.TwirpPackageIsVersion7`)
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems very similar to the protobuf assertion line, so I think this is a good idea to add so we can also validate the twirp version. LGTM!

@marioizquierdo marioizquierdo merged commit 01dd2f0 into master Sep 15, 2020
@marioizquierdo marioizquierdo deleted the version_constant_for_backwards_compatibility branch September 15, 2020 22:41
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