Support overriden types #116
Conversation
There was a problem hiding this comment.
Also, what happens if I run the generator twice in succession? Presumably the second time it will think that all the generated types are manually overriden? Perhaps you should only consider overrides when they're contained by a file not matching ^// Code generated .* DO NOT EDIT\.$.
I think we all agree that this is not a good long-term design, but as far as quick hacks go, I think it's an easy win for now. We can delete this if/when we come up with something nicer.
| types := make(map[string]struct{}) | ||
| for _, pack := range packs { | ||
| for fname, f := range pack.Files { | ||
| if strings.HasPrefix(path.Base(fname), "ipldsch_") { |
There was a problem hiding this comment.
I know this is a generator and the performance isn't the most important thing, but parsing all files to then only look at a small minority is a bit wasteful :)
It would be significantly less work to use globbing to find the ipldsch_*.go files we're interested in, and then parse and inspect those.
There was a problem hiding this comment.
This is excluding the three ipldsch_*.go files and looking at all others to find types that are defined not in generated code. (this also addresses your other comment of "is it stable over multiple runs")
There was a problem hiding this comment.
🤦 I read the logic backwards. I guess my point here is still valid, though now it's just "you can avoid reading+parsing these 3-4 files".
|
|
||
| // getExternTypes provides a mapping of all types defined in the destination package. | ||
| // It is used by generate to not duplicate defined types to allow overriding of types. | ||
| func getExternTypes(pth string) (map[string]struct{}, error) { |
|
One idea that came to me while you spoke at the standup: right now, if the code generator changes and you copied an older version of its generated code, you likely won't notice that you're using mismatching versions of generated code until they behave slightly wrong, or break in weird and confusing ways. If we want to merge this, I'd like to have some sort of sanity check that all the generated types in a single package belong to the same version. Ideally at compile time, via constant expressions :) |
Thinking about this again, it feels overkill given that it's a temporary hacky solultion, anyway. LGTM, and let's merge since my comments were minor too. |
This change will look at the destination package that codegen is being built into, and will skip generation of types that are already declared by files not prefixed with
ipldsch_.This isn't the cleanest escape-hatch, but it's a start.