Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jonjohnsonjr
Copy link
Contributor

This also hoists the trace logic from the build subcommand up into the root Command so that it's available for every subcommand.

luhring
luhring previously approved these changes May 22, 2024
imjasonh
imjasonh previously approved these changes Jul 8, 2024
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>
Comment on lines +133 to +134
cmd.PersistentFlags().StringVar(&traceFile, "trace", "", "where to write trace output")
_ = cmd.PersistentFlags().MarkHidden("trace")
Copy link
Contributor

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

Copy link
Contributor Author

@jonjohnsonjr jonjohnsonjr Jul 11, 2024

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.

Copy link
Member

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™️.

Copy link
Contributor

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".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants