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

allow setting version and revision #1368

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pendo324
Copy link
Contributor

@pendo324 pendo324 commented Sep 6, 2024

Issue #, if available:

Description of changes:
Allows setting the VERSION and REVISION variables for use in the Makefile, instead of always inferring them from the git repository. This is useful when building from the source tarball instead of a Git repository.

Testing performed:
Built a binary using VERSION=myversion REVISION=1234567 make soci soci-snapshotter-grpc and checked the output of --version commands:

./out/soci --version
soci version myversion 1234567

./out/soci-snapshotter-grpc --version
soci-snapshotter-grpc version myversion 1234567

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Signed-off-by: Justin Alvarez <alvajus@amazon.com>
@pendo324 pendo324 self-assigned this Sep 6, 2024
@pendo324 pendo324 requested a review from a team as a code owner September 6, 2024 19:50
@sondavidb
Copy link
Contributor

I wonder if we want any sort of redundancy check. For example if we were to accidentally ship a binary with an incorrect version attached, it might be good to mention that this version was manually set with some indicator of sorts.

Not sure if it's a silly concern or not, though. It definitely seems like an edge case that's not likely to be a big problem, but would like your thoughts on that anyway.

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.

4 participants