Add JsonnetVM methods for formatting #17
Conversation
There was a problem hiding this comment.
Thank you for your contribution and sorry for my late reply.
I'm glad that you volunteer to be a collaborator.
Overall I welcome these changes. But I just want to add some comments on style consistency (function name), impl details and API design (constants).
Co-authored-by: Yuki Yugui Sonoda <yugui@yugui.jp>
These namings are taken from google/go-jsonnet. https://pkg.go.dev/github.com/google/go-jsonnet@v0.16.0/formatter?tab=doc
ext/jsonnet/vm.c
Outdated
| rb_define_method(cVM, "fmt_file", vm_fmt_file, 2); | ||
| rb_define_method(cVM, "fmt_snippet", vm_fmt_snippet, 2); |
There was a problem hiding this comment.
Sorry. I overlooked this point at the first comment.
As long as you intend to call these methods from their wrappers, these methods should be private.
Should use rb_define_private_method as eval_file or eval_snippet does.
yugui
left a comment
yugui
left a comment
There was a problem hiding this comment.
LGTM except for the rb_define_private_method thing.
The public APIs are format_file and format respectively.
|
Oh, this PR is broken by updating jsonnet to v0.16. |
2 participants
No description provided.