Add support for quarkus:dev in multimodule projects #1973
Conversation
|
Looks like draft PR's don't get CI, changing to a normal one |
| resources.append(file.getAbsolutePath()); | ||
| res = file.getAbsolutePath(); | ||
| } | ||
| DevModeContext.ModuleInfo moduleInfo = new DevModeContext.ModuleInfo(getSourceDir().getAbsolutePath(), |
There was a problem hiding this comment.
getSourceDir() is not actually not "correct" atm. The QuarkusPluginExtension.sourceDir/outputDirectory() methods returns a File, but it's "misleading"/wrong. The SourceSets they extract those locations from return a FileCollection. When we send it as a String/File the actual content of it is "correct" when the FileCollection only contained one item, else it's will be: file1:file2:file3....
I think we should change the Extension methods to return a List instead.
There was a problem hiding this comment.
Would you be able to look into this after the initial maven support is merged? I have not really looked at Gradle at all.
| @@ -150,6 +150,8 @@ | |||
| <jetty.version>9.4.15.v20190215</jetty.version> | |||
| <flyway.version>5.2.4</flyway.version> | |||
| <yasson.version>1.0.3</yasson.version> | |||
| <!-- Used for integration tests, to make sure webjars work--> | |||
| <bootstrap.version>3.1.0</bootstrap.version> | |||
There was a problem hiding this comment.
Just to note that our bootstrap which is currently in the same repo is an independent project for which we may need to add another property here. So one or the other (or both) at that point will have to be disambiguated.
| break; | ||
| } | ||
| } | ||
| if (classLoc != null && Files.exists(classLoc)) { |
There was a problem hiding this comment.
Just a minor one, if it's not null we can assume it exists.
There was a problem hiding this comment.
We can't actually, would need to change the logic in the loop above to use a different variable and only assign after the test
| try { | ||
| return Files.exists(resourcePath) ? resourcePath.toUri() | ||
| return resourcePath != null && Files.exists(resourcePath) ? resourcePath.toUri() |
| //TODO: there is probably a better algorithm than this to establish the partial ordering | ||
| //basically we just iterate and add anything to the list that has not dependencies in 'toplace' | ||
| //this has worst case performance of O(n^2), if there is a linear relationship between | ||
| //the modules and the original order is reversed. As N is generally fairly small we can live with this for now |
There was a problem hiding this comment.
Theoretically, we might not need to include all the projects from the workspace. We could take the target project, navigate its dependencies and that should be it? I could look into that afterwards if you like.
There was a problem hiding this comment.
If the number of maven modules is big enough that the partial ordering algorithm becomes noticeable it would be a drop in the ocean. That TODO should be removed, I would be suprised if N is ever more than 100
There was a problem hiding this comment.
Yeah, it's not only the ordering though. They will be checked for changes, etc.
There was a problem hiding this comment.
ah, I see what you mean. I think as a first attempt it is ok to monitor the whole project and look at improving it later
stuartwdouglas
force-pushed
the
multimodule
branch
from
ebe6a09
to
899aba3
Compare
stuartwdouglas
force-pushed
the
multimodule
branch
from
899aba3
to
633906b
Compare
|
it looks good and whatever improvements we need in the maven/gradle plugins we can do in later prs. |
Marked as a draft as it still needs tests, but it would be good to get early feedback