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

maintain: set dev version #3294

Merged
merged 1 commit into from
Sep 26, 2022
Merged

maintain: set dev version #3294

merged 1 commit into from
Sep 26, 2022

Conversation

mxyng
Copy link
Collaborator

@mxyng mxyng commented Sep 23, 2022

Summary

  • Docker build without setting --build-arg=BUILDVERSION will produce version 99.99.99999
  • go build or equivalent commands will produce version 99.99.99999 unless LDFLAGS are set
  • CI sets RELEASE_NAME for artifact builds and BUILDVERSION for image builds so the version will be whatever is in those variables

Checklist

  • Wrote appropriate unit tests
  • Considered security implications of the change
  • Updated associated docs where necessary
  • Updated associated configuration where necessary
  • Change is backwards compatible if it needs to be (user can upgrade without manual steps?)
  • Nothing sensitive logged
  • Considered data migrations for smooth upgrades

Related Issues

Resolves #

Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

Nice! I like how this simplifies things.

Anything blocking merging this?

@mxyng
Copy link
Collaborator Author

mxyng commented Sep 23, 2022

Nothing in particular. It really depends if this addresses the problems @pdevine is trying to solve.

@pdevine
Copy link
Contributor

pdevine commented Sep 23, 2022

I like the simplification in version.go. Can you modify the Makefile to remove the excess version lines at the top of it? Those are effectively useless right now.

I am slightly nervous about development getting weird versions, however I think it's pretty obvious that you're getting a bogus value with this change. The one thing it doesn't address is whether your gh repo is stale, although that's not entirely addressed by the other change either. On other projects I have worked on we have included something like: 18a3cd029d79f17 (stale) after the version which might be a nice addition to this.

@mxyng mxyng marked this pull request as ready for review September 26, 2022 17:39
@mxyng mxyng merged commit 4925dc0 into main Sep 26, 2022
@mxyng mxyng deleted the mxyng/dev-verison branch September 26, 2022 17:58
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