Resubmit #149 with fixes #182
Conversation
| @@ -43,12 +44,10 @@ | |||
| * then we consider the value in {@code <branch>} tag as a command to run and the output will be used as the actual branch */ | |||
| @Parameter(property = "format.branch", defaultValue = "master") | |||
| private String branch; | |||
| @Parameter(readonly = true, defaultValue = "${project}") | |||
| @Parameter(property = "project", readonly = true, required = true, defaultValue = "${project}") | |||
There was a problem hiding this comment.
compared to #149: added property = "project" as some online sources insist that it's required with the switch from maven-project to maven-core.
however, i also tested with setting only one of property = "project" and defaultValue = "${project}", and all three versions seem to work. looks like this was not the culprit, but decided to keep both just in case.
| @@ -0,0 +1,54 @@ | |||
| package org.antipathy.mvn_scalafmt | |||
There was a problem hiding this comment.
compared to #149: moved the bulk of the logic from FormatMojo.getSources to this source, along with unit tests.
|
|
||
| private def getCanonicalFile(dir: String): File = { | ||
| val relfile = new File(dir) | ||
| val absfile = if (relfile.isAbsolute) relfile else new File(project.getBasedir, dir) |
There was a problem hiding this comment.
compared to #149: the check for .isAbsolute looks to be the primary reason why that commit failed. when testing that build locally, i must not have observed that the list of formatted files was empty (because i didn't expect it).
it took me a while to track this down; I suspected that the move to maven-core was the culprit, not this method; neither user who commented on #158 included the full error message (rather than ...), or it would have been obvious that the project directory had been repeated.
|
|
||
| private def appendAlternative(altDirs: JList[String], outputDir: String): Unit = { | ||
| val outputPath = getCanonicalFile(outputDir).toPath | ||
| sources ++= altDirs.asScala.map(getCanonicalFile).filter(!_.toPath.startsWith(outputPath)) |
There was a problem hiding this comment.
compare to #149: added the logic excluding generated files (those under the output dirs).
kitbellew
force-pushed
the
149
branch
2 times, most recently
from
72aa341 to
df35228
Compare
Make sure to filter out those falling under output dirs.
| project.getBasedir().toPath.resolve(dir).toFile().getCanonicalFile() | ||
|
|
||
| private def getCanonicalFile(dir: File): File = | ||
| project.getBasedir().toPath.resolve(dir.toPath).toFile().getCanonicalFile() |
There was a problem hiding this comment.
changed to Path.resolve because of codacy. this replaces the code that failed in #149 because of unexpected behaviour of new File(a, b).
|
PR appears to be stale |
|
generally ready for review |
Description
Fixes #158.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Checklist: