-
Notifications
You must be signed in to change notification settings - Fork 120
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
Add hidden gcptrace flag #1230
base: main
Are you sure you want to change the base?
Add hidden gcptrace flag #1230
Conversation
This also hoists the trace logic from the build subcommand up into the root Command so that it's available for every subcommand. Signed-off-by: Jon Johnson <jon.johnson@chainguard.dev>
cmd.PersistentFlags().StringVar(&traceFile, "trace", "", "where to write trace output") | ||
_ = cmd.PersistentFlags().MarkHidden("trace") |
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 we're marking this as hidden, could we have some documentation somewhere to show that it exists and how to use it? It's an incredible developer feature, IMHO
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 think basically nobody but me uses it, and I'd like to be able to break it in the future.
I'd also be fine un-hiding it if you think we should be more serious professionals.
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 like to explicitly not commit to supporting this right now, which means (unfortunately) not telling anybody outside of this completely private circle of trust that it exists.
If it proves as useful as we think it is, and word catches on, we should un-hide it and document it.
I think a more ideal outcome would be that all the GCP trace exporting happens in a different, non-melange-CLI caller that wraps melange-as-a-library, since running the melange CLI in GCP directly as a long-term strategy is Bonkers™️.
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 agree with that more ideal outcome. And I agree that we shouldn't overcommit to supporting something externally.
I'm okay with leaving it hidden. I'd like to make a non-blocking request that we at least document this internally, if not now then soon. I'd love to move away from things that only one person gets to know about, even when the thing is not yet considered "stable".
This also hoists the trace logic from the build subcommand up into the root Command so that it's available for every subcommand.