Re: [PR] Set the default source directory to `src/${scope}/${lang}` as per documentation [maven]

[via GitHub]

elharo commented on code in PR #2180:
URL: https://github.com/apache/maven/pull/2180#discussion_r2010479726


##########
api/maven-api-core/src/main/java/org/apache/maven/api/SourceRoot.java:
##########
@@ -26,13 +26,23 @@
 /**
  * A root directory of source files.
  * The sources may be Java main classes, test classes, resources or anything 
else identified by the scope.
+ *
+ * <h2>Default values</h2>
+ * The default implementation of all methods in this interface match the 
default values
+ * documented in {@code maven.mdo} (the <abbr>POM</abbr> model).
  */
 public interface SourceRoot {
     /**
      * {@return the root directory where the sources are stored}.
      * The path is relative to the <abbr>POM</abbr> file.
+     *
+     * <h4>Default implementation</h4>
+     * The default value is <code>src/{@linkplain #scope() scope}/{@linkplain 
#language() language}</code>

Review Comment:
   main is not a scope. Is that OK?



##########
api/maven-api-model/src/main/mdo/maven.mdo:
##########
@@ -875,7 +875,7 @@
           <description>
             All the sources to compile and resources files to copy for a 
project or it's unit tests.
             The sources can be Java source files, generated source files, 
scripts or resources for examples.
-            Each source is specified by a mandatory {@code directory} element, 
which is relative to the POM.
+            Each source is specified by a {@code directory} element, which is 
relative to the POM.
             The kind of sources (codes to compile or resources to copy) and 
their usage (for the main code

Review Comment:
   codes --> code or perhaps source files



##########
api/maven-api-model/src/main/mdo/maven.mdo:
##########
@@ -2005,7 +2005,7 @@
         <![CDATA[
         Description of the sources associated with a project main code or unit 
tests.
         The sources can be Java source files, generated source files, scripts 
or resources for examples.

Review Comment:
   scripts,



##########
api/maven-api-core/src/main/java/org/apache/maven/api/SourceRoot.java:
##########
@@ -62,8 +72,11 @@ default ProjectScope scope() {
 
     /**
      * {@return the language of the source files}.
+     * The default value is {@link Language#JAVA_FAMILY}.

Review Comment:
   Just use the constant value here. No need to hide this behind a variable in 
the docs.



##########
api/maven-api-core/src/main/java/org/apache/maven/api/SourceRoot.java:
##########
@@ -26,13 +26,23 @@
 /**
  * A root directory of source files.
  * The sources may be Java main classes, test classes, resources or anything 
else identified by the scope.
+ *
+ * <h2>Default values</h2>
+ * The default implementation of all methods in this interface match the 
default values
+ * documented in {@code maven.mdo} (the <abbr>POM</abbr> model).

Review Comment:
   I don't quite follow this, might need a bit of rewrite to clarify



##########
api/maven-api-model/src/main/mdo/maven.mdo:
##########
@@ -2005,7 +2005,7 @@
         <![CDATA[
         Description of the sources associated with a project main code or unit 
tests.
         The sources can be Java source files, generated source files, scripts 
or resources for examples.
-        A source is specified by a mandatory {@code directory} element, which 
is relative to the POM.
+        A source is specified by a {@code directory} element, which is 
relative to the POM.
         The directory content can optionally by reduced to a subset with the 
{@code includes} and

Review Comment:
   by --> be



##########
api/maven-api-model/src/main/mdo/maven.mdo:
##########
@@ -2005,7 +2005,7 @@
         <![CDATA[
         Description of the sources associated with a project main code or unit 
tests.
         The sources can be Java source files, generated source files, scripts 
or resources for examples.
-        A source is specified by a mandatory {@code directory} element, which 
is relative to the POM.
+        A source is specified by a {@code directory} element, which is 
relative to the POM.

Review Comment:
   This seems confusingly similar to source files.



##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java:
##########
@@ -65,24 +65,26 @@ public final class DefaultSourceRoot implements SourceRoot {
      * @param source a source element from the model
      */
     public DefaultSourceRoot(final Session session, final Path baseDir, final 
Source source) {
-        String value = nonBlank(source.getDirectory());
-        if (value == null) {
-            throw new IllegalArgumentException("Source declaration without 
directory value.");
-        }
-        directory = baseDir.resolve(value);
-        FileSystem fs = directory.getFileSystem();
+        FileSystem fs = baseDir.getFileSystem();
         includes = matchers(fs, source.getIncludes());
         excludes = matchers(fs, source.getExcludes());
         stringFiltering = source.isStringFiltering();
         enabled = source.isEnabled();
         moduleName = nonBlank(source.getModule());
 
-        value = nonBlank(source.getScope());
+        String value = nonBlank(source.getScope());
         scope = (value != null) ? session.requireProjectScope(value) : 
ProjectScope.MAIN;
 
         value = nonBlank(source.getLang());

Review Comment:
   We should not be reusing these local variables for different things



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org