-
Notifications
You must be signed in to change notification settings - Fork 331
cmd/cue: add --outfile flag to get go
#4118
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
base: master
Are you sure you want to change the base?
Conversation
56697cf
to
3a72ca5
Compare
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.
largely SGTM but needs a test. for example, copy get_go_vendor.txtar
into get_go_outfile.txtar
and adapt as you see fit. ideally we test the multi-file package scenario as well.
3a72ca5
to
2ffd695
Compare
Ah forgot the test, ignore the request for review. Sorry ^^' |
This introduces a way to write generated cue schemas to a given file or stdout. It joins all declarations into one giant file and preserves all package comments. Signed-off-by: Tim Windelschmidt <[email protected]>
2ffd695
to
a7ac603
Compare
Now with added test 😄 |
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.
Thanks!
) | ||
for i, f := range p.Syntax { | ||
// If we are in the first iteration we need to set cuePkg. | ||
// Af we want to join everything into the same file, |
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.
typo: "As"?
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.
(ignore given my suggested rewrite below)
// If we are in the first iteration we need to set cuePkg. | ||
// Af we want to join everything into the same file, | ||
// we have to keep the same cuePkg and declarations around | ||
// for all iterations, else reset. |
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.
I'd move this comment above the for loop, and also explain what's happening with writing only the last (joined) file. Something like...
// By default, we output one CUE file for each Go file as long as there's anything to generate.
// When --outfile is used, we want to generate exactly one CUE file for an entire Go package,
// so we instead keep joining declarations until we reach the last Go file, where we then write.
-- cue.mod/module.cue -- | ||
module: "mod.test" | ||
language: version: "v0.9.0" | ||
-- x/x.go -- |
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.
best to include multiple files in package x
, to make sure that the joining of declarations is tested.
|
||
if flagOutFile.IsSet(cmd) && len(pkgs) != 1 { | ||
return errors.New("--outfile only allows for one package to be specified") | ||
} |
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.
I'd test these edge cases in the testscript too, like...
! exec cue get go --outfile out.cue --local ./x
stderr 'mutually exclusive'
! exec cue get go --outfile out.cue ./x ./y
stderr 'only allows for one package'
if !flagOutFile.IsSet(e.cmd) { | ||
for pkgPath := range e.usedPkgs { | ||
if !e.done[pkgPath] { | ||
e.done[pkgPath] = true | ||
if err := e.extractPkg(root, e.allPkgs[pkgPath]); err != nil { | ||
return err | ||
} |
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.
if !flagOutFile.IsSet(e.cmd) { | |
for pkgPath := range e.usedPkgs { | |
if !e.done[pkgPath] { | |
e.done[pkgPath] = true | |
if err := e.extractPkg(root, e.allPkgs[pkgPath]); err != nil { | |
return err | |
} | |
for pkgPath := range e.usedPkgs { | |
if !flagOutFile.IsSet(e.cmd) && !e.done[pkgPath] { | |
e.done[pkgPath] = true | |
if err := e.extractPkg(root, e.allPkgs[pkgPath]); err != nil { | |
return err |
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.
there's a negligible performance difference either way, so there's no need to add a level of nesting here
# We can only compare the generated file to the expected golden file, | ||
# as all imports are skipped and only the given package is generated. | ||
cmp outfile.cue x_go_gen.cue.golden | ||
|
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.
perhaps test that we didn't generate any of the usual files too, like...
! exists cue.mod/gen/ x/x_go_gen.cue y/y_go_gen.cue
This introduces a way to write generated cue schemas to a given file or stdout. It joins all declarations into one giant file and preserves all package comments.
Closes #4108