Closed davideme closed 2 months ago
@raphael all suggestions applied. Happy to see it's not far off what was expected.
Any update ?
Sorry for the delay, changes look great! Just one last question: I see a number of places where nil
is given as argument for the metadata. This means that these files will still have the version number in the header, is that what we want?
@tchssk I update the Meta name. @raphael I review all the calls to Header,
.Meta
The changes look great, thank you for the contribution!
Could you please wait a moment? I'm worried about backward compatibility.
@tchssk let me know which kind of scenario you are thinking? Having the meta tag in a previous version of goa?
Before merging, we are doing some additional testing in our own codebase of around 600 goa generated files.
Adding arguments to codegen.Header()
breaks compatibility. Normally this function is only used internally in Goa, but I'm wondering if there is another way to solve the problem if possible.
Adding arguments to
codegen.Header()
breaks compatibility. Normally this function is only used internally in Goa, but I'm wondering if there is another way to solve the problem if possible.
- Adding a cli flag
- Implement as a plugin
Good point! A third option could be to make the extra parameter optional (e.g. using the functional options pattern). We've avoided adding CLI flags in the past and I'm not sure this would be worth the trade-off.
I prefer plugins to keep the core simple, but optional parameters aren't bad either.
After testing extensively in our code base, using Meta tag seems impractical. it will fill the design files with tag for each kind of files. I'll explore the plugin way.
I created a plugin. Here you are. https://github.com/tchssk/goaplugins/tree/master/goaversionremover
When Updating goa version it's hard to detect the files that are only changing the version number and the ones that are changing code.
I'm proposing adding a Meta Field "goa:version:disable" that would disable the version for this specific section.
Remarks: