Add flatten on eval #104
Add flatten on eval #104
Conversation
| msg, ok := AsProtoMessage(maybeMsg) | ||
| if !ok { | ||
| return nil, fmt.Errorf("%q returned something that's not a protobuf (a %s)", parsedOpts.funcName, maybeMsg.Type()) | ||
| // Only flatten but not flatten deep. This will flatten out, in order, lists within main list and append the |
There was a problem hiding this comment.
To avoid breaking any other existing uses of skycfg... would you be against making an ExecOption to turn on list flattening? I imagine just add a flattenLists bool to the execOptions struct, and then a:
// WithFlattenLists flattens lists one layer deep ([1, [2,3]] -> [1, 2, 3])
func WithFlattenLists() ExecOption {
return fnExecOption(func(opts *execOptions) {
opts.flattenLists = true
})
}
Then you can case the logic below on parsedOpts.flattenLists
| caseName: "flatten nested list", | ||
| fileToLoad: "test13.sky", | ||
| expProtos: []proto.Message{ | ||
| &pb.MessageV2{ |
There was a problem hiding this comment.
then add a flattenLists field to endToEndTestCase which passes WithFlattenLists() as an argument to config.Main() and set it to true here
garvinp-stripe
requested review from
jwog-stripe and
dl-stripe
and removed request for
jwog-stripe
skycfg_test.go
Outdated
| if testCase.flattenLists { | ||
| return config.Main(context.Background(), skycfg.WithVars(testCase.vars), skycfg.WithFlattenLists()) | ||
| } else { | ||
| return config.Main(context.Background(), skycfg.WithVars(testCase.vars)) | ||
| } | ||
|
|
There was a problem hiding this comment.
Can we make the flattenLists property in the test cases an execOptions slice. We can then provide skycfg.WithFlattenLists() as a value in the test case and pass in the testCase.execOptions... in the call and forego the if/else statement.
type endToEndTestCase struct {
...
execOptions []skycfg.ExecOption
}
testCases := []endToEndTestCase{
...
endToEndTestCase{
caseName: "flatten nested list",
...
execOptions: []skycfg.ExecOption{skycfg.WithFlattenLists()},
},
}
return config.Main(context.Background(), skycfg.WithVars(testCase.vars), testCase.execOptions...)
Add ability to flatten nested list on evaluations with WithFlattenLists option.
Fix build due to deprecated zlib 1.2.11
bazelbuild/rules_proto#117