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

If docker.image.tag is set to a placeholder, we should replace it. #1469

Merged

Conversation

henri-tremblay
Copy link
Contributor

@henri-tremblay henri-tremblay commented May 5, 2021

Fix #1468
This occurs when docker.image.name is constructed with ${docker.image.repo}:${docker.image.tag}.

This is a fix to #1468

@henri-tremblay
Copy link
Contributor Author

Note: It is undocumented that docker.image.tag can override %l. So if I do mvn docker:build -Ddocker.image.name=%a:%l -Ddocker.image.tag=nightly I will end up with myapp:nightly

@codecov
Copy link

codecov bot commented May 5, 2021

Codecov Report

Merging #1469 (7a29f39) into master (61e0cc7) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 7a29f39 differs from pull request most recent head 390b3d3. Consider uploading reports for the commit 390b3d3 to get more accurate results

@@             Coverage Diff              @@
##             master    #1469      +/-   ##
============================================
+ Coverage     61.94%   61.97%   +0.02%     
- Complexity     2149     2150       +1     
============================================
  Files           166      166              
  Lines          9484     9488       +4     
  Branches       1441     1439       -2     
============================================
+ Hits           5875     5880       +5     
+ Misses         3095     3093       -2     
- Partials        514      515       +1     
Impacted Files Coverage Δ Complexity Δ
.../fabric8/maven/docker/util/ImageNameFormatter.java 95.83% <100.00%> (+1.71%) 11.00 <0.00> (ø)
...8/maven/docker/config/BuildImageConfiguration.java 81.25% <0.00%> (ø) 88.00% <0.00%> (ø%)

@rohanKanojia
Copy link
Member

@henri-tremblay: Thanks a lot for the PR. Could you please add a line to doc/changelog.md regarding this change?

Note: It is undocumented that docker.image.tag can override %l. So if I do mvn docker:build -Ddocker.image.name=%a:%l -Ddocker.image.tag=nightly I will end up with myapp:nightly

Shall we document this behavior in this PR or in separate PR?

@henri-tremblay
Copy link
Contributor Author

Done for the changelog. You should really generate the change log from the issues in the release. I'm doing something similar here: https://github.com/easymock/easymock/blob/master/generate-changelog.sh.

I will let you update the documentation in another PR. From my reading of ImageNameFormatter, two parameters are special. docker.image.user and docker.image.tag. Both can be passed in parameter to override the %g and %v %t %l.

@rohanKanojia rohanKanojia added this to the 0.36.0 milestone May 15, 2021
…is occurs when docker.image.name is constructed with ${docker.image.repo}:${docker.image.tag} (fabric8io#1468)
@rohanKanojia rohanKanojia merged commit f0d189b into fabric8io:master May 15, 2021
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.

A placeholder in docker.image.tag isn't replaced by the final result when used during docker:build
2 participants