Skip to content

Fix --version output - #1352

Merged
timvaillancourt merged 2 commits into
github:masterfrom
timvaillancourt:fix-version-output
Dec 11, 2023
Merged

timvaillancourt merged 2 commits into
github:masterfrom
timvaillancourt:fix-version-output

Conversation

@timvaillancourt

@timvaillancourt timvaillancourt commented Dec 11, 2023 •

Copy link
Copy Markdown
Collaborator

Description

This PR fixes a problem introduced in #1347 due to my forgetting to add newline character to fmt.Printf(...) 🤦

Secondly, I've made an empty GitCommit display a bit more clearer

Before (notice no \n char)

tvaillancourt@tvailla-ltmxctu gh-ost % ./gh-ost --version
unversioned (git commit: )%  

After: undefined AppVersion+GitCommit

tvaillancourt@tvailla-ltmxctu gh-ost % ./gh-ost --version                        
unversioned (git commit: unknown)
tvaillancourt@tvailla-ltmxctu gh-ost %

After

tvaillancourt@tvailla-ltmxctu gh-ost % go build -o gh-ost -ldflags "-X main.AppVersion=1.1.6 -X main.GitCommit=$(git rev-parse HEAD)" ./go/cmd/gh-ost/main.go
tvaillancourt@tvailla-ltmxctu gh-ost % ./gh-ost --version                                                                                                    
1.1.6 (git commit: 5ebb95347e2002a5a959483f3e801cd3267e7e88)
tvaillancourt@tvailla-ltmxctu gh-ost %

In case this PR introduced Go code changes:

  • contributed code is using same conventions as original code
  • script/cibuild returns with no formatting errors, build errors or unit test errors.

Signed-off-by: Tim Vaillancourt <tvaillancourt@slack-corp.com>
Signed-off-by: Tim Vaillancourt <tvaillancourt@slack-corp.com>
Comment thread go/cmd/gh-ost/main.go
flag.PrintDefaults()
return
}

@timvaillancourt timvaillancourt Dec 11, 2023 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Let's always fallback to unversioned/unknown so that the starting gh-ost %+v logline later also gets this fallback instead of ""

@timvaillancourt
timvaillancourt requested review from a user, meiji163 and rashiq December 11, 2023 16:08
@timvaillancourt
timvaillancourt marked this pull request as ready for review December 11, 2023 16:08
@timvaillancourt timvaillancourt added this to the v1.1.7 milestone Dec 11, 2023
@timvaillancourt
timvaillancourt merged commit 679d39e into github:master Dec 11, 2023
@timvaillancourt
timvaillancourt deleted the fix-version-output branch December 11, 2023 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants