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 --version option to the rootCmd #301

Closed
wants to merge 3 commits into from
Closed

Conversation

N4r35h
Copy link

@N4r35h N4r35h commented Apr 29, 2024

resolves issue #298

its already built into cobra just needed to add the Version string to the rootCommand

image

@guergabo
Copy link
Contributor

guergabo commented Apr 29, 2024

Hi @N4r35h,

Thanks for this! I was thinking of using go's build process for this issue instead and pass the versioning information to the linker via ldflags. That way it's not so static and we can configure it externally from a github workflow. Otherwise, we have to modify the source code for each build.

It could default to dev or something of the sorts.

Feel free to explore this route and update this PR. Thanks.

@N4r35h
Copy link
Author

N4r35h commented Apr 30, 2024

sure @guergabo,
i have made the changes please refer to the image and let me know if we should add something else or change things around

i added the vsc.version (automatically set by go build) which is the commit hash also feeling it might be useful where we distribute dev versions of the build for testing and want to correlate to the particular commit also seen it done by the docker cli, we can add more info such as version of golang it was compiled with and built time if required

could you explain about the github release workflow or confirm if thats being used right now ?

go build -ldflags="-X github.com/resonatehq/resonate/cmd/version.Version=v0.5.0"

image

var Version string = "dev"

var VersionCmd = &cobra.Command{
Use: "version",
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to make a whole command out of this a variable should do. check this out: https://www.digitalocean.com/community/tutorials/using-ldflags-to-set-version-information-for-go-applications

cmd/root.go Outdated
@@ -42,6 +43,7 @@ func init() {
rootCmd.AddCommand(dst.NewCmd())
rootCmd.AddCommand(serve.ServeCmd())
rootCmd.AddCommand(quickstart.NewCmd())
rootCmd.AddCommand(version.VersionCmd)
Copy link
Contributor

@guergabo guergabo Apr 30, 2024

Choose a reason for hiding this comment

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

the behavior should still remain using the cobra version flag: resonate --version

@guergabo
Copy link
Contributor

regarding the github release workflow, it starts here: https://github.com/resonatehq/resonate/blob/main/.github/workflows/release.yaml

let me take a look in the morning and suggest some follow up actions, because we would want the version information passed to both our docker image release build & github release artifact build

@guergabo
Copy link
Contributor

guergabo commented Apr 30, 2024

ok, for docker image release build:

for github release build:

@N4r35h
Copy link
Author

N4r35h commented Apr 30, 2024

thanks for pointing out the flake.nix file

i have made the requested changes consider it a draft let me know if anything else is required
i will look into setting this into the workflow in a while

)

func computeVersion() string {
Copy link
Contributor

Choose a reason for hiding this comment

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

check out the link i sent before, i'll paste here again: https://www.digitalocean.com/community/tutorials/using-ldflags-to-set-version-information-for-go-applications

this whole function is not needed.

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