Michael, first of all thanks for your conceptual thoughts and efforts to improve both code and documentation.
> Let me know what you think! I fully agree that the notion that both the output directory and the report output directory are meant to be *base* directories. I disagree, however, with that they should default to the same base directory. Hence https://github.com/apache/maven-reporting-impl/pull/26. I am more in the Maven Javadoc "fan club" in this case. Regards -- Alexander Kriegisch https://scrum-master.de Michael Osipov schrieb am 23.10.2023 00:53 (GMT +07:00): > Alexander, > > I have now read the code of all our reporting plugins, > maven-reporting-impl and maven-site-plugin. > > Here is my understanding how I expect to work which it is not at the > moment and there is a flaw in the system were both MJXR and MJAVADOC > apply a hack and I will explain why they do. > > We have > > standalone execution: >> @Parameter(defaultValue = "${project.reporting.outputDirectory}", >> readonly >> = true, required = true) >> protected File outputDirectory; > > and > > site execution: >> @Parameter(property = "siteOutputDirectory", defaultValue = >> "${project.reporting.outputDirectory}") >> protected File outputDirectory; > > both point to root directory where all reports put there files in, from > the simplest single page one, to multipage to those which are external > and operate in subdirectories. Everything below that output directory is > fixed and completely subject to the plugin. > > Both o.a.m.reporting.AbstractMavenReport.getReportOutputDirectory() and > o.a.m.reporting.AbstractMavenReport.setReportOutputDirectory(File) exist > for the sole purpose to accomondate both execution styles. One value is > exected through DI (standalone) the other one set (through site). > Therefore, in both cases #getReportOutputDirectory() should be used to > get the right value to write to. > In any case #setReportOutputDirectory(File) does *not* contain a > directory for the reporting plugin only, but for the entire generated > tree of plugins, regardless of 'mvn site' /or/ 'mvn pmd:pmd > checkstyle:checkstyle'. > > Now, o.a.m.reporting.MavenReport.getOutputName() has bad documentation > because it is actually a relative output *path* to the generated item > with the base/root path given by > o.a.m.reporting.MavenReport.getReportOutputDirectory(). Again, here is > the name completely deceiving. Needs doc update as well. > > You might wonder, what to do now with: >> @Override >> public String getOutputName() { >> return "xref/index"; >> } > > and > >> @Override >> public void setReportOutputDirectory(File reportOutputDirectory) { >> if ((reportOutputDirectory != null) >> && >> (!reportOutputDirectory.getAbsolutePath().endsWith("xref"))) >> { >> this.destDir = new File(reportOutputDirectory, >> "xref").getAbsolutePath(); >> } else { >> this.destDir = reportOutputDirectory.getAbsolutePath(); >> } >> } > ? > > Well, while I consider the former is correct, the latter is not: > * Does one expect to pass *null* as reportOutputDirectory? > * Why is *this.reportOutputDirectory* never updated? > * It expects the report this live under *.../xref*, but ignores >> /** >> * Folder where the Xref files will be copied to. >> */ >> @Parameter(defaultValue = "${project.reporting.outputDirectory}/xref") >> private String destDir; > * It mixes the the notion of a *pluginReportOutputDirectory* which is > passed to the external report generator as a root/base or a multi sink > if you want to generate multiple reports under one dir, but have the > starting point at *foo/index* (output name) and all resides under *foo/*. > MJAVADOC does the same trick although it has replaces the literal 'xref' > with a field, but the issue remains the same. > Whether the path component(s) between the root and *index* should be > flexible, is upto the plugin (e.g, xref, xref-test, apidocs, etc.), but > from my personal PoV changing it does not make: > * In site execution this has to be carried along > * If you need to change it otherwise you can rename/move it with an ant > task or a subprocessor. > > What I will do not is to create a few issues and PRs to improve the > situation and create a PR for JXR on top of 'doxia-2.0.0' how I consider > it should be. Some of the method names are unfortunately, but I am > afraid it is too hard to change them given they have too widespread use. > > I guess after all, I will need to perform another round of releases with > those PRs before I can start new reporting plugin releases. I think what > you have raised was long overdue and absoltuely legit. > > Let me know what you think! > > @Hervé, I would also appreciate to hear your opinion on this! > > Michael > > --------------------------------------------------------------------- > To unsubscribe, e-mail: users-unsubscr...@maven.apache.org > For additional commands, e-mail: users-h...@maven.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: users-unsubscr...@maven.apache.org For additional commands, e-mail: users-h...@maven.apache.org
