Skip to content

Conversation

exoego
Copy link
Contributor

@exoego exoego commented Aug 22, 2025

HTTP client construction fails mostly when GCP credential is not configured.
This guide will improve deck first-timer experience.

@Songmu
Copy link
Collaborator

Songmu commented Aug 23, 2025

It would certainly be useful to display such a guide, so I would like to do that.
However, I don't think using fmt.Print to output to standard output is a very good idea.
That said, the logger is currently customized, and there is no policy in place for how such messages should be displayed to users in the development of deck.

@exoego
Copy link
Contributor Author

exoego commented Aug 24, 2025

I used the logger initially.
But it does not print log unless --verbose.
And its format is JSON object, which is not the best human-readable format on terminal.

@Songmu
Copy link
Collaborator

Songmu commented Aug 24, 2025

You are right. I have filed an issue for resolution under #393.

@Songmu
Copy link
Collaborator

Songmu commented Aug 25, 2025

We should probably use (*cobra.Command).Print, but since we can't use it in the part you are changing, we need to think of a way to handle it properly in the upper layer.

@Songmu
Copy link
Collaborator

Songmu commented Aug 25, 2025

I thought it would be better to use cmd.Print when deck.New, deck.Create, or deck.CreateFrom in ./cmd return an error.
This change will result in more than one line being modified, and instruction messages will be displayed even for errors other than client creation errors, but this approach is better because it is more loosely coupled.
You are welcome to make this change yourself, or I can do it for us.

@Songmu
Copy link
Collaborator

Songmu commented Aug 25, 2025

It's done, right? It looks good, so I'll merge it. Thank you!

@Songmu Songmu merged commit 0f782c1 into k1LoW:main Aug 25, 2025
1 check passed
@github-actions github-actions bot mentioned this pull request Aug 25, 2025
@exoego exoego deleted the guide branch August 25, 2025 11:05
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.

2 participants