Skip to content

Remove commit hash from Version string#1021

Merged
jetstack-bot merged 3 commits intocert-manager:masterfrom
nalum:997-commit-in-version
Oct 29, 2018
Merged

Remove commit hash from Version string#1021
jetstack-bot merged 3 commits intocert-manager:masterfrom
nalum:997-commit-in-version

Conversation

@nalum
Copy link
Contributor

@nalum nalum commented Oct 27, 2018

What this PR does / why we need it:
This PR removes the Git commit hash from the version string unless github.com/JetStack/cert-manager/pkg/util.AppVersion is set to canary.

Which issue this PR fixes: fixes #997

Special notes for your reviewer:
Open question on issue #997.

Release note:

Removed the Git commit hash from the version string in non `canary` builds (#997)

Only add the commit hash to the version string if the AppVersion is set
to canary. See cert-manager#997

Signed-off-by: Luke Mallon (Nalum) <luke@mallon.io>
@jetstack-bot jetstack-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 27, 2018
@jetstack-bot jetstack-bot requested a review from wallrj October 27, 2018 16:38
@jetstack-bot jetstack-bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 27, 2018
Signed-off-by: Luke Mallon (Nalum) <luke@mallon.io>
@jetstack-bot jetstack-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 27, 2018
@munnerz
Copy link
Member

munnerz commented Oct 29, 2018

/ok-to-test

@jetstack-bot jetstack-bot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 29, 2018
@munnerz
Copy link
Member

munnerz commented Oct 29, 2018

(copying your question over from the issue):

👍 @munnerz Is this something that should be applied to the version string in general or just the user agent?
While tags are a pointer to a specific commit there is the possibility that a tag has been moved (for what ever reason) so maybe only applied to the user agent string.

I think applying it to the version in general should suffice. We won't be changing git tags after they are created, and if we do, presumably there is a very good reason and the other version has not rolled out yet.

Let's go with how you've done it here, and if we start hitting issues we can adjust accordingly 😄

Thanks! This is perfect, and thanks also for the unit tests!

You'll need to run bazel run //hack:update-bazel and commit the change BUILD.bazel file in order to have this pass the codegen and unit checks 😄

/approve

@jetstack-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: munnerz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jetstack-bot jetstack-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 29, 2018
@nalum
Copy link
Contributor Author

nalum commented Oct 29, 2018

@munnerz I will get on that as soon as I can 👍

@munnerz
Copy link
Member

munnerz commented Oct 29, 2018

Thanks @nalum! I think a simple bazel run //hack:update-bazel should sort these errors out!

Signed-off-by: Luke Mallon (Nalum) <luke@mallon.io>
@jetstack-bot jetstack-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 29, 2018
@nalum nalum changed the title [WIP] Remove commit hash from Version string Remove commit hash from Version string Oct 29, 2018
@jetstack-bot jetstack-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 29, 2018
@munnerz
Copy link
Member

munnerz commented Oct 29, 2018

Thanks again!

/lgtm

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 29, 2018
@jetstack-bot jetstack-bot merged commit 86ef1db into cert-manager:master Oct 29, 2018
@nalum nalum deleted the 997-commit-in-version branch October 29, 2018 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't include git commit ref in user agent for tagged releases

3 participants