Allow disabling version string in code generated header #3509
Conversation
codegen/header.go
Outdated
| return &SectionTemplate{ | ||
| Name: "source-header", | ||
| Source: headerT, | ||
| Data: map[string]any{ | ||
| "Title": title, | ||
| "ToolVersion": goa.Version(), | ||
| "ToolVersion": toolVersion, |
There was a problem hiding this comment.
Just to be consistent maybe this should be disableVersion and the test in the template should be {{ if not .DisableVersion }}?
codegen/sections_test.go
Outdated
| @@ -68,6 +68,16 @@ import ( | |||
| package testpackage | |||
|
|
|||
| `, goa.Version()) | |||
| titleHeaderDisableVersion = `// Code generated by goa, DO NOT EDIT. | |||
There was a problem hiding this comment.
Maybe we call this titleHeaderVersionDisabled to make it a qualified name?
codegen/service/client.go
Outdated
| header := codegen.Header(service.Name+" client", svc.PkgName, imports) | ||
| value, ok := service.Meta.Last("goa:version:disable") | ||
| disableVersion := ok && value == "true" | ||
| header := codegen.Header(service.Name+" client", svc.PkgName, imports, disableVersion) |
There was a problem hiding this comment.
I'm wondering if we wouldn't be better of passing the metadata object to codegen.Header and moving the logic that computes disableVersion there:
- This would remove the need for having this computation spread everywhere
- This would allow for other metadata based tweaks in the future when generating file headers
|
@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 |
codegen/header.go
Outdated
| goa "goa.design/goa/v3/pkg" | ||
| ) | ||
|
|
||
| // Header returns a Go source file header section template. | ||
| func Header(title, pack string, imports []*ImportSpec) *SectionTemplate { | ||
| func Header(title, pack string, imports []*ImportSpec, meta expr.MetaExpr) *SectionTemplate { | ||
| value, ok := meta.Last("goa:version:disable") |
There was a problem hiding this comment.
Meta("goa:version:generate", "false") might be better.
It's a similar syntax to Meta("openapi:generate", "false").
|
@tchssk I update the Meta name.
|
|
The changes look great, thank you for the contribution! |
|
Could you please wait a moment? |
|
@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
|
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 created a plugin. Here you are. |
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: