gengo: support for unions with stringprefix representation. #133
Conversation
warpfork
force-pushed
the
gengo-union-repr-stringprefix
branch
from
024f852
to
4da73fb
Compare
| // SplitN is an alias of strings.SplitN, which is only present here to | ||
| // make it usable in codegen packages without requiring conditional imports | ||
| // in the generation process. | ||
| func SplitN(s, sep string, n int) []string { |
There was a problem hiding this comment.
hmm, just run goimports on the generated code? "figure out what imports we need" is a solved problem in go :)
There was a problem hiding this comment.
That seems significantly more complex. I'm not okay with pushing a need for goimports -- something from the x/tools repos -- onto users in order to have compilable code come out. (By contrast, while elsewhere we decided we're okay leaning on post-gen invocation of gofmt, that differs on two fronts: it's effectively always available, and it's also not strictly necessary to apply before compile will succeed.)
A small handful of cherry-picked constants like this seems fine to me.
We may want to consider moving such things to their own small package (or the minima file in the output?) to make them less obtrusive, though.
There was a problem hiding this comment.
I guess goimports is probably also possible to use as a library, so we could just add it to our own dependencies. But I'd still say for this small of a need, it's probably not worth it. I'd favor KISS on this until we really have a desire for conditional imports.
There was a problem hiding this comment.
I was under the assumption we'd be the only ones running this generator. Why would other people generate their own mixins directly?
There was a problem hiding this comment.
Maybe the comment is unclear -- SplitN is used in the parsing logic of the code that comes out.
| TypeSymbol: adjCfg.TypeSymbol(typ), | ||
| }, | ||
| pkgName, | ||
| typ, |
There was a problem hiding this comment.
use keyed fields? otherwise it's hard to tell what's going on here. I personally always use keys when the number of fields is more than 2
Title says it all, actually.