Conversation
|
|
|
Thanks for the contribution, is there any chance to update the summary of the PR to get more context on this please? |
|
I updated the top comment, is that what you need? |
Dogild
left a comment
Dogild
left a comment
There was a problem hiding this comment.
Please update the summary of the diff with examples of this new feature. Can you also please update the Changelog with this new feature
| case "json": | ||
| opts.ROpts.Encoding = encoding.JSON | ||
| case "protobuf": | ||
| opts.ROpts.Encoding = encoding.Protobuf |
There was a problem hiding this comment.
When using the option -e, --encoding, encoding.Protobuf will match with proto and not protobuf.
| return err | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
We should prevent users of using file and request together. Right now, File have priority over request, only one should be available
| case "raw": | ||
| opts.ROpts.Encoding = encoding.Raw | ||
| } | ||
|
|
There was a problem hiding this comment.
Line 169, there is the following line of code overrideParam(&opts.ROpts.RequestJSON, string(body)). How would it work when the body given in file has another format than JSON? Right now, body could be with format proto, thrift or raw.
| opts := newOptions() | ||
| err := readYAMLFile("testdata/valid.yab", nil, opts) | ||
| assert.NoError(t, err) | ||
| } |
There was a problem hiding this comment.
Can we add more test please? We should provide the following unit tests:
- Yab template with file + JSON format
- Yab template with file + YAML format
- Yab template with file + RAW format
- Yab template with file + Proto format
- Yab template with file + Thrift format
- Negative test with bad combinaison of options (like File and request together)
3 participants
Many of the options to yab can be specified in a YAML template. In my case I want to specify a file to read the request data from and also to specify how the request is encoded.
This diff enables these options within the template file.