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

Add filtering #777

Merged
merged 3 commits into from
May 16, 2017
Merged

Conversation

rhuss
Copy link
Collaborator

@rhuss rhuss commented May 16, 2017

Tune this with parameter <filter>.

Tune this with parameter `<filter>`.
Pattern propertyPattern =
Pattern.compile("(?<variable>" + Pattern.quote(delimiters[0]) + "(?<prop>.*?)" + Pattern.quote(delimiters[1]) + ")");
Matcher matcher = propertyPattern.matcher(line);
StringBuffer ret = new StringBuffer();
Copy link
Collaborator

Choose a reason for hiding this comment

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

MAJOR Replace the synchronized class "StringBuffer" by an unsynchronized one such as "StringBuilder". rule

private static String[] extractDelimiters(String filter) {
if (filter == null ||
filter.equalsIgnoreCase("false") ||
filter.equalsIgnoreCase("none")) {
return null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

MAJOR Return an empty array instead of null. rule

@codecov
Copy link

codecov bot commented May 16, 2017

Codecov Report

Merging #777 into master will decrease coverage by 0.04%.
The diff coverage is 52.87%.

@@             Coverage Diff              @@
##             master     #777      +/-   ##
============================================
- Coverage     47.46%   47.41%   -0.05%     
- Complexity     1054     1063       +9     
============================================
  Files           134      134              
  Lines          6835     6909      +74     
  Branches        888      904      +16     
============================================
+ Hits           3244     3276      +32     
- Misses         3316     3352      +36     
- Partials        275      281       +6
Impacted Files Coverage Δ Complexity Δ
.../io/fabric8/maven/docker/service/BuildService.java 30.07% <0%> (-0.46%) 9 <0> (ø)
...8/maven/docker/assembly/DockerAssemblyManager.java 12.76% <0%> (-1.66%) 6 <0> (ø)
...config/handler/property/PropertyConfigHandler.java 90.3% <100%> (+0.04%) 40 <0> (ø) ⬇️
...aven/docker/config/handler/property/ConfigKey.java 100% <100%> (ø) 7 <0> (ø) ⬇️
...a/io/fabric8/maven/docker/util/DockerFileUtil.java 79.59% <80%> (+19.59%) 13 <13> (+11) ⬆️
...8/maven/docker/config/BuildImageConfiguration.java 81.64% <88.88%> (-0.18%) 45 <1> (ø)
...ain/java/io/fabric8/maven/docker/util/EnvUtil.java 58.51% <0%> (-4.45%) 37% <0%> (-2%)

@pepperbot
Copy link
Collaborator

SonarQube analysis reported 4 issues

  • MAJOR 2 major
  • MINOR 2 minor

Watch the comments in this conversation to review them.

2 extra issues

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. MINOR BuildImageConfiguration.java#L440: Remove this use of "command"; it is deprecated. rule
  2. MINOR BuildImageConfiguration.java#L451: Remove this use of "command"; it is deprecated. rule

@rhuss
Copy link
Collaborator Author

rhuss commented May 16, 2017

self-[merge], don't wanna bother you with this feature. I will take the consequences for this :)

@rhuss
Copy link
Collaborator Author

rhuss commented May 23, 2017

@realmfoo The logic wrt with Dockerfiles is explained in the manual. By default, if dockerFile is given, and its a relative path, its looked up relatively to src/main/docker.

The reason why you can specify the Dockerfile name is, that you might have a single directory with multiple, slightly differing Dockerfile but the same files e.g. to COPY. So you can use different dockerFile configs in your image to reuse those files.

@realmfoo
Copy link

realmfoo commented May 23, 2017 via email

rgbj pushed a commit to rgbj/docker-maven-plugin that referenced this pull request Jun 21, 2017
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