-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat(isthmus): cli can now output JSON, TEXT and BINARY proto format #216
Conversation
Partial fix for: #211 |
Technically text is protobuf text. We are still looking to add the Substrait text format. |
cli can now output PROTOJSON, PROTOTEXT and BINARY proto format
28b8c11
to
60a6afb
Compare
@EpsilonPrime good point. What do you think about the names |
@vbarua thank you for working on this. |
That works for me. The existing implementation can take an explicit format
type and it can also auto detect the type so having some way to eventually
provide auto detect as an option would be nice as well.
…On Wed, Dec 27, 2023, 00:24 Vibhatha Lakmal Abeykoon < ***@***.***> wrote:
@vbarua <https://github.com/vbarua> thank you for working on this.
—
Reply to this email directly, view it on GitHub
<#216 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABDDCEQHWLGYLRRYJKAZULYLL23NAVCNFSM6AAAAABBCXPF3OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRZGY2DMNZWGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
If these changes look reasonable now, I would appreciate an approval when y'all have a chance 🙇 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vbarua LGTM. Thanks for working on this.
cli can now output PROTOJSON, PROTOTEXT and BINARY proto format * ci: update graalVersion
No description provided.