Refactor path config#128
Conversation
|
Please document the whole legacy format and matchers in spec and design document. |
| { | ||
| final List<String> patterns = this.config.get().getPathConfigs().stream() // | ||
| .map(PathConfig::getPattern) // | ||
| .map(PathConfig::getDescription) // |
There was a problem hiding this comment.
Variable name says "patterns" yet you see to be collecting "descriptions". Please unify.
| { | ||
| return pattern; | ||
| } | ||
| return GLOB_PREFIX + pattern; |
There was a problem hiding this comment.
This default behavior should appear in both specification and design.
|
|
||
| /** | ||
| * Create a new path configuration. | ||
| * Create a new pattern based path configuration. |
| */ | ||
| public PathConfig(final String pattern, final String coveredItemArtifactType, | ||
| final String coveredItemNamePrefix, final String tagArtifactType) | ||
| public static PathConfig createPatternConfig(final String pattern, |
There was a problem hiding this comment.
Four unnamed arguments. Replace factory method by builder?
| + ", coveredItemNamePrefix=" + this.coveredItemNamePrefix | ||
| + ", coveredItemArtifactType=" + this.coveredItemArtifactType + ", tagArtifactType=" | ||
| + this.tagArtifactType + "]"; | ||
| return "PathConfig [pathMatcher=" + this.pathMatcher.toString() + ", coveredItemNamePrefix=" |
There was a problem hiding this comment.
Maybe enclose the comma separated parts in quotes for better readability of the output?
| return this.description; | ||
| } | ||
|
|
||
| boolean matches(final Path path) |
There was a problem hiding this comment.
package scope on purpose?
There was a problem hiding this comment.
Yes. DescribedPathMatcher is only used in this package.
| this.matcher = matcher; | ||
| } | ||
|
|
||
| static DescribedPathMatcher createPatternMatcher(final String pattern) |
There was a problem hiding this comment.
Package scope on purpose?
| return new DescribedPathMatcher(fullPattern, patternMatcher); | ||
| } | ||
|
|
||
| static DescribedPathMatcher createPathListMatcher(final List<Path> paths) |
There was a problem hiding this comment.
Package scope on purpose?
| return GLOB_PREFIX + pattern; | ||
| } | ||
|
|
||
| String getDescription() |
There was a problem hiding this comment.
Package scope on purpose?
| * <http://www.gnu.org/licenses/gpl-3.0.html>. | ||
| * #L% | ||
| */ | ||
|
|
There was a problem hiding this comment.
Please add JavaDoc to all public methods. To package-scoped optionally - but recommended.
There was a problem hiding this comment.
This is only used internally. Users only work with PathConfig.
Codecov Report
@@ Coverage Diff @@
## develop #128 +/- ##
=============================================
- Coverage 92.53% 92.52% -0.01%
+ Complexity 814 813 -1
=============================================
Files 81 82 +1
Lines 2210 2235 +25
Branches 196 196
=============================================
+ Hits 2045 2068 +23
- Misses 122 125 +3
+ Partials 43 42 -1
Continue to review full report at Codecov.
|
2 participants
Allow configuring a list of paths instead of pattern