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

decide if we want to validate thrift on pull request or not #69

Closed
codefromthecrypt opened this issue Apr 30, 2019 · 4 comments
Closed
Labels

Comments

@codefromthecrypt
Copy link
Member

Unlike swagger and proto, there's no pure javascript tool to validate or use thrifts. You have to rely on a OS binary to first generate the code. This means our travis config either has to invoke two tools or we have a binary dependency in our validate.test.js only for thrift.

another option is to simply not bother with thrift checking as we don't add anything to it anymore.

cc @apache/zipkin-committers

@codefromthecrypt
Copy link
Member Author

fwiw we've never validated thrift on pull request, and incidentally until today even swagger validation was busted!

Now, we validate swagger and proto on pull request.

@jcchavezs
Copy link
Contributor

jcchavezs commented Apr 30, 2019 via email

@jeqo
Copy link
Member

jeqo commented May 1, 2019

I'm with @jcchavezs, if we don't add anything to thrift anymore, we should not add validation.

@codefromthecrypt
Copy link
Member Author

rule of three

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants