Skip to content

Conversation

alex-laties
Copy link

@alex-laties alex-laties commented Jun 26, 2019

solve for #2953

Allows VERSION and REVISION to be set via build args.

E.G docker build --build-arg VERSION=somerelease --build-arg REVISION=internalRev .

Signed-off-by: Alex Laties <agl@tumblr.com>
@codecov
Copy link

codecov bot commented Jun 26, 2019

Codecov Report

Merging #2955 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2955      +/-   ##
==========================================
- Coverage   60.59%   60.54%   -0.05%     
==========================================
  Files         102      102              
  Lines        8025     8025              
==========================================
- Hits         4863     4859       -4     
- Misses       2512     2515       +3     
- Partials      650      651       +1
Flag Coverage Δ
#linux 60.54% <ø> (-0.05%) ⬇️
Impacted Files Coverage Δ
registry/handlers/app.go 48.16% <0%> (-0.7%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90dfea7...92d213d. Read the comment docs.

@alex-laties
Copy link
Author

weird... i don't understand how I'm reducing code coverage in registry/handlers/app.go given I am not editing that file at all...

@alex-laties
Copy link
Author

@manishtomar @caervs is it possible to get eyes on this?

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM (not a maintainer)

Copy link
Contributor

@manishtomar manishtomar left a comment

Choose a reason for hiding this comment

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

LGTM. Would love to see a bash doc about ?=.

VERSION=$(shell git describe --match 'v[0-9]*' --dirty='.m' --always)
REVISION=$(shell git rev-parse HEAD)$(shell if ! git diff --no-ext-diff --quiet --exit-code; then echo .m; fi)
VERSION ?= $(shell git describe --match 'v[0-9]*' --dirty='.m' --always)
REVISION ?= $(shell git rev-parse HEAD)$(shell if ! git diff --no-ext-diff --quiet --exit-code; then echo .m; fi)
Copy link
Contributor

Choose a reason for hiding this comment

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

For my own understanding could you please link to bash doc or any page that describes that ?= means use variable value if provided otherwise calculate from command. I only see ${VAR:-20} when searching online. I may very well be dumb in looking.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Sure. TL;DR ?= assigns the expression to the variable if and only if the variable is undefined in the environment. If defined and empty, ?= will not assign the expression. The expression is lazily evaluated recursively on use, similar to =.

?= docs

It's a surprisingly lesser known feature but extremely useful. Handles cases where you wish for VERSION or REVISION to be intentionally blanked out.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤦‍♂ Its a makefile change. Thanks for the doc!

@caervs caervs merged commit 10f7263 into distribution:master Jul 11, 2019
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