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

include Cortex version in cortex_build_info #2468

Merged
merged 2 commits into from
Apr 17, 2020

Conversation

thorfour
Copy link
Contributor

@thorfour thorfour commented Apr 15, 2020

What this PR does: Adds the version,branch,revision to the build args, so that cortex_build_info correctly populates the version number.

Fixes #1709

cortex_build_info{branch="build-info-version",goversion="go1.13.3",revision="ca93c1b87",version="1.0.0"} 1

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@thorfour thorfour force-pushed the build-info-version branch 2 times, most recently from c2437ac to e79b3f7 Compare April 15, 2020 16:19
@bboreham
Copy link
Contributor

Thanks for this PR, definitely needed! Should add "fixes #1709".

We had some discussion in #1751 why ./tools/image-tag might be a better choice.

@thorfour thorfour force-pushed the build-info-version branch from e79b3f7 to ca93c1b Compare April 16, 2020 15:03
@thorfour
Copy link
Contributor Author

I've update the Makefile to use the short revision format, but I don't think there's any value in using ./tools/image-tage since that combines the fields we want.

Copy link
Contributor

@bboreham bboreham left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Thanks! I much prefer this solution, compared to handling different linker paths based on vendoring or not.

@pstibrany
Copy link
Contributor

Shall we have CHANGELOG entry for this?

Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Thanks @thorfour for working on it! I just pushed the CHANGELOG entry so we can move forward with the PR.

Thor and others added 2 commits April 17, 2020 11:01
Signed-off-by: Thor <thansen@digitalocean.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
@pracucci pracucci force-pushed the build-info-version branch from cbe7ab2 to c7c5e8a Compare April 17, 2020 09:02
@pracucci pracucci merged commit 996cb83 into cortexproject:master Apr 17, 2020
@thorfour thorfour deleted the build-info-version branch April 17, 2020 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Startup message can have blank version
4 participants