refactor(api): use structs #376
Conversation
This converts all of the public API to no longer take a wild variety of modifier functions, but instead clearly defined structs that only allow setting those properties, that are actually relevant to that specific struct. This not only reduces the amount of boilerplate required, but also makes the API more discoverable and defines it more clearly. THIS IS A BREAKING CHANGE
Adapts all commands to the new struct based API style
There was a problem hiding this comment.
i'm usually a little wary of type embedding when there's methods involved - usually, people are trying to simulate class inheritance and end up having a bad day - but flattening pure config/opts-shaped value objects is a good case for it.
And, as you noted, use of the variadic opts pattern here wasn't great, as (i'm inferring? didn't actually verify) it allowed for config values to be passed to some commands that were not applicable to the command. i tend to reserve use of that pattern to situations where a) every possible value implementing the opt type is applicable to functions that receive the opt, and b) the behavior being controlled by the opts is variable enough that the struct just feels (subjectively) like a random grab bag.
👍
| opts.apply.AutoApprove = b | ||
| } | ||
| } | ||
| type JsonnetOpts = jsonnet.Opts |
There was a problem hiding this comment.
Are you type aliasing so that the embedded property on Opts is named Opts.JsonnetOpts instead of Opts.Opts?
There was a problem hiding this comment.
Yes! And also because I prefer the surface of this package to only be consisted of itself, so consumers do not also need to import pkg/jsonnet.
Only pkg/tanka is considered public-facing, while the others are more-or-less internal packages that are public so others could hack on them.
Refactors our API away from the Modifier pattern to plain old struct arguments.
Not quite sure who to assign for review on this. This is intended as a refactor in the strict sense, being a no-op