[MNG-8713] SourceRoot.directory() default value should include the module when present #2278
Conversation
| @@ -79,7 +79,12 @@ public DefaultSourceRoot(final Session session, final Path baseDir, final Source | |||
| if (value != null) { | |||
| directory = baseDir.resolve(value); | |||
| } else { | |||
| directory = baseDir.resolve("src").resolve(scope.id()).resolve(language.id()); | |||
| var src = baseDir.resolve("src"); | |||
There was a problem hiding this comment.
Do we use var in maven codebase? I kinda remember we agreed not to? (but I may be missing some later agreement).
There was a problem hiding this comment.
Right, it was my mistake.
Personally, I sometime use var in expressions such as var x = new Foo() when there is no way for the type to be something else than what we can read on the line, and usually refrain to use it in other contexts. I do not know if there is a Maven policy.
There was a problem hiding this comment.
I strongly prefer to avoid var. IMHO it should never have been added to the language. I want to know the types of things when I'm reading the code. var hides that info. I also don't want method return types to change without explicit consideration and inspection of every call point.
…name when present.
desruisseaux
force-pushed
the
module-in-default-directory
branch
from
71b9748 to
90933fe
Compare
elharo
changed the title
| * The default value depends on whether a {@linkplain #module() module name} is specified in this source root: | ||
| * <ul> | ||
| * <li> | ||
| * If no module name, then the default directory is |
There was a problem hiding this comment.
shifting from main/java to java/main feels like a really big change. Is there a very compelling reason to do this?
There was a problem hiding this comment.
Reminder: this pull request does not change the convention of non-modular projects or projects that do not use the module source hierarchy: for them, the default stay src/main/java. Only for projects that use the module source hierarchy, we would "flip" from (nothing that exist currently in Maven) to src/java/org.my.module/main.
The reason for putting ${lang} before ${module} is because the module names tend to be language-dependent. For example, Java uses URL domain names (e.g. org.apache.maven) while Python uses shorter module names, without org or com prefix. If a Maven subproject mixes Java and Python modules, placing the modules of the two languages in the same directory is confusing.
The reason for putting ${scope} after ${module} is for grouping together the files related to each module. This is the same division as Maven subprojects (previously named "modules"): each subproject contains main, test, resource and pom.xml files. Likewise, each Java module would contain its own main, test, resource files, with module-info somewhat playing the role of pom.xml. In other words, we are still splitting the projects in modules, but shifting from a world where modules are managed by Maven to a world were modules are managed by Java.
There is precedents: ${module}/${scope} is the directory layout of NetBeans's Ant modular project, of Christian Stein's blog (except for ${lang}) and of the example given in Oracle's documentation (except for src and ${lang}).
There was a problem hiding this comment.
That makes sense to me. @elharo ?
There was a problem hiding this comment.
I am not convinced. That feels very wrong to me, and I expect users will be unnecessarily confused. What we've had in the past is src/main/java/package-structure and the natural extension of that is src/main/java/module/package-structure
If other languages come into play (which I really don't think will happen or is something we need to worry about) then src/main/python/module or whatever
Changing anything to src/java/main introduces inconsistency we don't have now.
There was a problem hiding this comment.
I'm not sure that's the right angle to look at.
If a given project has multiple modules, those modules kinda act as subprojects, so it makes sense to group their files together, rather than splitting main / tests. In that case, each test directory is testing the code of the module, so having them close to each other makes more sense to me.
There was a problem hiding this comment.
Indeed, the rational is as Guillaume said. While we try to avoid disruption for users who are not interested in JPMS, those who want to embrace fully JPMS will have to change some habits anyway, some of them out of our control (e.g., the directory layout of javac output).
There is a difficulty with resources, which are considered as a language and therefore would be stored apart with a src/${lang}/${module}/${scope} layout if the users do not specify the directory themselves. An alternative staying closer to current Maven practice may be src/${module}/${scope}/${lang}.
There was a problem hiding this comment.
This layout is closer to the usual maven layout and may be less disruptive. It just adds the module before the scope. I like it better actually.
There was a problem hiding this comment.
The inconvenient of the alternative proposal is that if a subproject has two or more languages, the modules of different languages will be mixed in the same directory. However, it may not be important if, as suggested in other comments, we choose to not let the multi-languages issue influences the decision.
Note: this difficulty come from the fact that in the current non-modular layout, resources is implicitly understood as "resources of the Java artifact". It is not really an independent "language".
If there is no objection or alternative proposal, I will change the layout to src/${module}/${scope}/${lang} in a few days.
|
Maybe introduce a new modules directory and put all the non-defallt modules there? E.g. modules/{module-name}/src/... That is, today's src/... would contain only code for the default module, no others. |
I don't think you could mix both in the same maven subproject, so not sure that would help. |
|
Since this is a major user facing design decision, this should be written up in a detailed proposal, not in code, and sent to the dev list for broader discussion. Code can come after there's reasonable consensus on the design. |
|
An advantage of |
|
Okay, I will write a document. Do we have a wiki or something like that for this kind of discussion? |
I confirm. When Java Module Source Hierarchy is used, there is no default module. |
If a
<source>element in POM does not provide any value for the<directory>element, the default value is currently defined assrc/${scope}/${lang}. This default value provides the familiarsrc/main/javaandsrc/test/javadefault directories. However, if the<source>element contains a<module>element, the module name must appear somewhere in the default directory name, otherwise the source code of multi-modular projects will collide.Different conventions are possible:
src/${module}/for the main code and a bazar of directories for the tests with no obvious order.src/${module}/for main code and does not mention tests.src/main/${module}/for the main code andsrc/test/${module}/for the test code. However, it also usesmodule-info.javain test code, which I would like to discourage.src/${module}/mainfor the main code andsrc/${module}/testfor the test code.I propose the NetBeans's convention, which is implemented in this pull request as a default directory of
src/${lang}/${module}/${scope}when the<module>element is present.Alternative
If it is considered too premature to adopt a default directory name for modular project, we should at least throw an exception asking user to specify a directory explicitly if a
<module>element is present.