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

Use docker.skip.tag property on push and remove #954

Merged
merged 2 commits into from
Apr 3, 2018
Merged

Use docker.skip.tag property on push and remove #954

merged 2 commits into from
Apr 3, 2018

Conversation

MrMarkW
Copy link

@MrMarkW MrMarkW commented Feb 22, 2018

When pushing and removing images, the docker.skip.tag property is not
used and can cause errors because the plugin will try to push and/or
remove image tags that don't exist because they were skipped during the
build process.

#869

Fix bug in removing tagged images

Code to remove tagged images was not creating the image name correctly

Signed-off-by: Mark Whelan mbwhelan@gmail.com

@MrMarkW
Copy link
Author

MrMarkW commented Feb 22, 2018

@rhuss - Here is the PR for #869

@codecov
Copy link

codecov bot commented Feb 22, 2018

Codecov Report

Merging #954 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master     #954      +/-   ##
============================================
- Coverage     51.67%   51.65%   -0.02%     
  Complexity     1351     1351              
============================================
  Files           147      147              
  Lines          7439     7441       +2     
  Branches       1121     1123       +2     
============================================
  Hits           3844     3844              
- Misses         3228     3230       +2     
  Partials        367      367
Impacted Files Coverage Δ Complexity Δ
...c/main/java/io/fabric8/maven/docker/BuildMojo.java 0% <ø> (ø) 0 <0> (ø) ⬇️
.../main/java/io/fabric8/maven/docker/RemoveMojo.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...rc/main/java/io/fabric8/maven/docker/PushMojo.java 0% <0%> (ø) 0 <0> (ø) ⬇️
.../fabric8/maven/docker/service/RegistryService.java 64% <0%> (-0.87%) 11 <0> (ø)

* Skip building tags
*/
@Parameter(property = "docker.skip.tag", defaultValue = "false")
protected boolean skipTag;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we move this to the Mojos only where it makes sense (Build/Remove/Push) ? This leads to some duplication, true, but makes it clear that its not usable in other Mojos.

Copy link
Collaborator

@rhuss rhuss left a comment

Choose a reason for hiding this comment

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

Makes sense to me, thanks !

Could you please also ...

  • ... update the documentation (in the configuration section for the build/push/remove)
  • ... and an entry to changelog.md ?

merci ...

@MrMarkW
Copy link
Author

MrMarkW commented Mar 26, 2018

@rhuss - I made the requested updates to code, docs, and changelog.

When pushing and removing images, the docker.skip.tag property is not
used and can cause errors because the plugin will try to push and/or
remove image tags that don't exist because they were skipped during the
build process.

Fixed bug in removing tagged images

Code to remove tagged images was not creating the image name correctly

Updated documentation and changelog

Signed-off-by: Mark Whelan <mbwhelan@gmail.com>
Copy link
Collaborator

@rhuss rhuss left a comment

Choose a reason for hiding this comment

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

Thanks ! LGTM

@rhuss rhuss merged commit b05a350 into fabric8io:master Apr 3, 2018
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.

3 participants