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 cli flag --version to show version of spanner-cli #198

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

kyu08
Copy link
Contributor

@kyu08 kyu08 commented Dec 2, 2024

Hello. Thank you for maintaining very useful cli for spanner!

What

SSIA

Why

It is more useful for troubleshooting if spanner-cli can show its version.

Implementation

I intentionally added Version to spannerOptions, though it might be not good idea to do so because it seems that --version is not a option to execute spanner-cli, but a kind of sub-command.

The reason why I did so is that I feel it is difficult to show description of --version if I don't add Version to spannerOptions.

As an alternative, I considered to parse os.Args like below. But as I mentioned above it it difficult to show description of --version when spanner-cli --help is executed.

	if len(os.Args) == 2 && os.Args[1] == "--version" {
		fmt.Fprintf(os.Stdout, "%s\n", versionInfo())
		os.Exit(0)
	}

Please give me your opinion.

Copy link
Collaborator

@yfuruyama yfuruyama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for sending this PR.

if !ok {
return "(devel)"
}
return info.Main.Version
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work? It looks like info.Main.Version still always returns (devel)...? https://groups.google.com/g/golang-nuts/c/ObM-ilI7sio

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing.🙏

This way is used by yo and wrench and they return their version properly.

It seems that it shows the valid version when it is released or installed via go install but I don't understand its mechanism. I'll look into it later.

Copy link
Collaborator

@apstndb apstndb Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: I believe it should be only valid in go install with tags.
Released binary should use another mechanism like LDFLAGS. (spanner-cli recently don't have release binaries).

I think you all are Japanese so I introduce this article.
https://rhysd.hatenablog.com/entry/2021/06/27/222254

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I didn't know that. Then maybe we can merge this PR and see how it goes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like pkg.go.dev correctly recognizes the version of spanner-cli from git tag: https://pkg.go.dev/github.com/cloudspannerecosystem/spanner-cli

FYI: I believe it should be only valid in go install with tags.

Do you mean go install github.com/cloudspannerecosystem/spanner-cli@latest always shows latest? Or does it show the git tag associated with the latest git commit?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you all for the information! The official installation of spanner-cli is go install as mentioned in README, so it looks like we don't need to manually embed the version information.

If it's fine, I'll go ahead and merge this PR.

Copy link
Collaborator

@apstndb apstndb Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean go install github.com/cloudspannerecosystem/spanner-cli@latest always shows latest? Or does it show the git tag associated with the latest git commit?

Oh, that was not precise answer.
It always shows module version when go install-ed, so it can print version even if not tagged.(go install *@latest use latest released module version)

$ go install github.com/cloudspannerecosystem/wrench@9432313  
go: downloading github.com/cloudspannerecosystem/wrench v1.10.2-0.20241120031843-9432313f606f
$ wrench --version
v1.10.2-0.20241120031843-9432313f606f

$ go install github.com/cloudspannerecosystem/wrench@latest 
go: downloading github.com/cloudspannerecosystem/wrench v1.10.1
$ wrench --version
v1.10.1

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, sounds good.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

image

@yfuruyama yfuruyama self-requested a review December 4, 2024 03:56
@yfuruyama yfuruyama merged commit a09f4ef into cloudspannerecosystem:master Dec 4, 2024
2 checks passed
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.

3 participants