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 command #157

Closed
wants to merge 1 commit into from
Closed

Add version command #157

wants to merge 1 commit into from

Conversation

teddylear
Copy link

@teddylear teddylear commented Mar 28, 2021

Adding version command to show current version of gotests installed.

NOTE Had issue with tests failing intermittently locally (even on develop). Not sure what the issue, please let me know if this is a known issue or something I should make another ticket for. This only appears to be happening for the 1.16.x build.

#133

@googlebot googlebot added the cla: yes Contributor signed Google CLA label Mar 28, 2021
@coveralls
Copy link

coveralls commented Mar 28, 2021

Coverage Status

Coverage remained the same at 95.619% when pulling d30b59f on teddylear:feature/add-version-command into 90387e1 on cweill:develop.

@teddylear teddylear marked this pull request as ready for review March 28, 2021 00:12
@butuzov
Copy link
Contributor

butuzov commented Mar 28, 2021

I wonder who is going to remember to bump the version manually? And Git commit? Maybe it's a nice time to add goreleaser to the project, and automate the build?

@teddylear
Copy link
Author

@butuzov As they way things are now it would have to be a manual bump prior to release. Ideally there would be a way to automate this (I can look into goreleaser). Also let me know about git commit, I can try to add that as well.

@butuzov
Copy link
Contributor

butuzov commented Mar 28, 2021

git commit is what @cweill was talking in #133 (comment)

@teddylear
Copy link
Author

@butuzov I have added code to add the commit on this branch, however the issue is that it requires the following build to inject the git commit hash.

export GIT_COMMIT=$(git rev-parse HEAD) && \
  go build -ldflags "-X github.com/cweill/gotests/gotests/process.GitCommit=$GIT_COMMIT"

As I'm new to golang, is there a way to set a default build script/build options when calling go build? I attempted searching for something like this but came up empty-handed. I'll keep looking but if you have any ideas of a better way to implement this/ build this let me know!

@butuzov
Copy link
Contributor

butuzov commented Mar 29, 2021

  1. maybe it's worth moving out the version functionality from process.go. its quite short and independent to be passed down to the stream. maybe it's better to locate in some file let's call it version.go under gotests alongside main.go.
  2. injecting git commit: usually done with a Makefile, Taskfile, or something similar. One thing you don't need to forget that somebody can build an app without a build system in place (therefore your go code should work with from git build or go install). I have mention goreleaser above, it can be added to CI and run automatically once software tagged. Here is a small example of how goreleser config.

so work plan is next:

  1. add version.go, and way to show version + including git hash it was built (testing it with manual builds).
  2. test default case (devel)
  3. add Makefile to automate git/version injecting
  4. add CI step.

@cweill can maybe add anything he would like to see in the implementation of this feature, as an author and maintainer.

@teddylear
Copy link
Author

@butuzov Ok I can look into this, but because go install needs to be supported then I cannot use the flags I mentioned? Else this will not work with go get correct?

@butuzov
Copy link
Contributor

butuzov commented Mar 30, 2021

@butuzov Ok I can look into this, but because go install needs to be supported then I cannot use the flags I mentioned? Else this will not work with go get correct?

Good that you asked! I learned this recently and haven't tested it yet. Usually, version should support "devel builds", with a fallback to something like v0.0.0-devel (git commit if possible)

@cweill
Copy link
Owner

cweill commented Mar 30, 2021

@teddylear Thanks for this PR. I agree with @butuzov, we should aim to automate the version number increasing. I'm not familiar with the best way to do this in Go, but there must be some precedence out there.

@teddylear
Copy link
Author

Let me keep working on this, but I'll close this PR for now while implementing these changes.

@teddylear teddylear closed this Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Contributor signed Google CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants