Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Howdy!
In the course of learning this code, I found a few tiny changes
that I hope you might find useful! I'm happy to split these into
separate pull requests. Or, please feel free to close this if you
don't want drive-by changes like these.
Thank you!
It seems the previous code intended to interpret the error from
rootCmd.ExecuteContext() as an *exec.ExitError. That's not the way
it worked in practice, though. Instead, the typecheck on
err.(*exec.ExitError) used the error from the call to tracer.Start().
This is due to the scoping rules of if statements.
With this commit, cmd.Do() now attempts to interpret the command
execution error as an *exec.ExitError. It also prints the error
message to stderr, since the caller in ./cmd/sqlc.main doesn't do
that.
See: https://go.dev/ref/spec#Blocks
Previously, some callers of trace.StartRegion() guarded on the
global debug.Traced variable. This guard is unneeded because
runtime/trace.StartRegion() will return a no-op region if tracing
has not been started. That means we can centralize the decision of
whether we want tracing to cmd.Do(). Since we then only have one
reference to debug.Traced, we can just delete it, since it seems
it was a shorthand for (debug.Debug.Trace != "").
See: https://cs.opensource.google/go/go/+/refs/tags/go1.19.3:src/runtime/trace/annotation.go;l=153-155
This patch silences the output of "go vet ./...", which used to say this:
Also, add go vet to CI.
Errors are handled by the root command's execution, so we don't
need to os.Exit() in any child commands.