[ https://issues.apache.org/jira/browse/MSHARED-1327?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17778892#comment-17778892 ]
Alexander Kriegisch edited comment on MSHARED-1327 at 10/24/23 2:03 AM: ------------------------------------------------------------------------ [~michael-o]: Another topic is the Javadoc for {{outputDirectory}} in both {{AbstractMavenReport}} and Maven Site: Would you agree that the term *base* directory and some additional explanation might be helpful for users? Site's javadoc is very terse anyway. It does not even explain where the default {{${project.reporting.outputDirectory}}} comes from - I myself have no idea, actually - and what default value it has. How about something like this? {code:java} public abstract class AbstractMavenReport extends AbstractMojo implements MavenMultiPageReport { /** * The output base directory for the report. Note that this parameter is only evaluated if the goal is run directly * from the command line. If the goal is run indirectly as part of a site generation, the output base directory * configured in the <a href="https://maven.apache.org/plugins/maven-site-plugin/site-mojo.html#outputDirectory"> * Maven Site Plugin</a> is used instead. * <p> * To the respective base directory for each use case (direct mojo call vs.site generation), implementing plugins * might want to add their specific subdirectories for multi-page reports, either using a hard-coded name or, * ideally, an additional user-defined mojo parameter with a default value. */ @Parameter(defaultValue = "${project.build.directory}", readonly = true, required = true) protected File outputDirectory; {code} Would you accept such a commit on top of my existing PR? was (Author: kriegaex): [~michael-o]: Another topic is the Javadoc for {{outputDirectory}} in both {{AbstractMavenReport}} and Maven Site: Would you agree that the term *base* directory and some additional explanation might be helpful for users? Site's javadoc is very terse anyway. It does not even explain where the default {{${project.reporting.outputDirectory}}} comes from - I myself have no idea, actually - and what default value it has. How about something like this? {code:java} public abstract class AbstractMavenReport extends AbstractMojo implements MavenMultiPageReport { /** * The output base directory for the report. Note that this parameter is only evaluated if the goal is run directly * from the command line. If the goal is run indirectly as part of a site generation, the output base directory * configured in the <a href="https://maven.apache.org/plugins/maven-site-plugin/site-mojo.html#outputDirectory"> * Maven Site Plugin</a> is used instead. * <p> * To the respective base directory for each use case (direct mojo call vs.site generation), implementing plugins * might want to add their specific subdirectories for multi-page reports, either using a hard-coded name or, * ideally, an additional user-defined mojo parameter with a default value. */ @Parameter(defaultValue = "${project.build.directory}", readonly = true, required = true) protected File outputDirectory; {code} Would you accept such a commit on top of my existing PR? > Change output directory default in AbstractMavenReport > ------------------------------------------------------ > > Key: MSHARED-1327 > URL: https://issues.apache.org/jira/browse/MSHARED-1327 > Project: Maven Shared Components > Issue Type: Improvement > Components: maven-reporting-impl > Affects Versions: maven-reporting-impl-4.0.0-M11 > Reporter: Alexander Kriegisch > Assignee: Michael Osipov > Priority: Major > Fix For: maven-reporting-impl-4.0.0-M12 > > > The output directory should default to {{${project.build.directory}}} instead > of {{${project.reporting.outputDirectory}}}. Quoting my reasoning from > https://github.com/apache/maven-jxr/commit/ae81a34ac616bf7e4ea4fa9d4eb09f0979eaccb1#commitcomment-130639906: > {quote} > (...) because the latter is simply wrong IMO. For stand-alone mojo execution, > a plugin should not mess with Maven Site's default directory. Imagine a > situation in which each module should get its own, self-consistent report > when called stand-alone, but the site should contain an aggregate with a > different structure or maybe no report from that plugin at all. The default > would then pollute the site directory. This is why on the ML I asked you > ([~michaelo]), if overriding a {{@Parameter}} annotation on a base class > field by another {{@Parameter}} annotation on a setter in a subclass it is > the right or canonical way to implement such a default override. > BTW, like I also said before, Maven Javadoc Plugin, even though it does not > use the abstract base class, implements the default correctly: build dir for > stand-alone, report dir in site generation context. > {quote} > The javadoc is, BTW, still correct and does not need to be changed: In a site > generation context, the reporting base directory will be set here > automatically without any further changes due to: > {code:java} > @Override > public void setReportOutputDirectory(File reportOutputDirectory) { > this.reportOutputDirectory = reportOutputDirectory; > this.outputDirectory = reportOutputDirectory; > } > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)