-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(cli codegen): fixed missing imports #3046
Conversation
c1df1a9
to
76cace6
Compare
Built on top of #3045 |
|
||
// look for default config (OS-specific, e.g. ".config" on linux) | ||
configDir, err = os.UserConfigDir() | ||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: indent
* fixes go-swagger#2969 This PR addresses an issue with goimports not being able to find another generated package. When generating files in 2 packages simultaneously (i.e. cli imports models and files are being generated model per model in both packages), there are situations when goimports fails to properly resolve the imported package (i.e. the imported package is not completed and may have errors while generating). This fix proceeds in 2 steps: first generate all models, then proceed with a final generation step ("PostModels") that iterates again over all models. It is a bit slower, but imports are now safe. Other additions with this PR: * updated dependency to golang.org/x/tools * added go-openapi/strfmt to default imports (was missing and added by goimports, but it is best to make it explicit and avoid unnecessary import fixes) * in templates, added standard library imports * relinted code generated by the CLI templates (check for errors, unused args, shadowed variables, prefer // over block comments, blank lines...) * updated cli.gotmpl to use os.UserConfigDir() and os.HomeDir() rather than importing a third party package * added guard for multiline .Description in calls to PersistentFlags() Tests: * added CLI codegen tests for go-swagger#2969, go-swagger#2650 and the latest docker spec (used to generate go-swagger/dockerctl) Signed-off-by: Frederic BIDON <fredbi@yahoo.com> reindented cli.gotmpl Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #3046 +/- ##
==========================================
- Coverage 82.67% 82.64% -0.03%
==========================================
Files 62 62
Lines 12818 12856 +38
==========================================
+ Hits 10597 10625 +28
- Misses 1685 1692 +7
- Partials 536 539 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This PR addresses an issue with goimports not being able
to find another generated package.
When generating files in 2 packages simultaneously (i.e. cli imports models
and files are being generated model per model in both packages),
there are situations when goimports fails to properly resolve the imported
package (i.e. the imported package is not completed and may have errors
while generating).
This fix proceeds in 2 steps: first generate all models, then proceed with
a final generation step ("PostModels") that iterates again over all
models. It is a bit slower, but imports are now safe.
Other additions with this PR:
goimports, but it is best to make it explicit and avoid unnecessary
import fixes)
args, shadowed variables, prefer // over block comments, blank lines...)
than importing a third party package
Tests:
(used to generate go-swagger/dockerctl)
Signed-off-by: Frederic BIDON fredbi@yahoo.com