-
Notifications
You must be signed in to change notification settings - Fork 399
fix: version command #135
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
fix: version command #135
Conversation
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 am trying out this branch and I am getting the following:
~/Documents/Personal/Go/test
❯ go-blueprint version
Go Blueprint CLI version (devel)
Is this expected? I am unsure what i did
|
||
"github.com/spf13/cobra" | ||
) | ||
|
||
// GoBlueprintVersion is the version of the cli to be overwritten by goreleaser in the CI run with the version of the release in github | ||
var GoBlueprintVersion string | ||
|
||
func getGoBlueprintVersion() string { | ||
if len(GoBlueprintVersion) != 0 { | ||
return GoBlueprintVersion |
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.
We can probably define this inline and not have a separate variable declared on line 14
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.
But GoBlueprintVersion needs to be on package level to be able to be set by the ldflag in .goreleaser.yml?
Or am I misunderstanding you?
cmd/version.go
Outdated
if info, ok := debug.ReadBuildInfo(); ok { | ||
return info.Main.Version | ||
} | ||
return "unknown" |
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.
Nit: Maybe we can return something else rather than "unknown" since that is a big ambiguous
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.
true, will try to come up with something better
Thoughts on these changes?
Also for additional info about getting (devel) from local builds with debug.ReadBuildInfo() there are these related issues on the golang repo: |
@limesten I am still investigating this but it seems it still does not work |
I assume you are checking out the pr and then running go install. I've not found a good way to include the version info in this case. It will work if:
The weird thing with 2) is that u cant't rly test it locally beforehand (as far as I can see at least) cuz this PR first needs to be included in a release so we can install it with |
@Melkeydev This is working. I created a test repo, pushed the tag, the build passed and the version was correct. LGTM |
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.
LGTM
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.
Problem/Feature
the command
go-blueprint version
works fine when downloading the pre-build binary from a release.However, if doing
go install github.com/melkeydev/go-blueprint@latest
it seems to result in an emptyGoBlueprintVersion
string.Description of Changes:
lost a few hairs on my head investigating this but:
getGoBlueprintVersion
If the version var has been set by goreleaser it will use that
else we will use
ReadBuildInfo
from the runtime/debug package (which gets the version from git tags)also added a fallback which just prints version unknown
Checklist