You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "hboutemy (via GitHub)" <gi...@apache.org> on 2024/03/06 07:18:57 UTC
[PR] MPLUGIN-511 add plugin requirements history detection [maven-plugin-tools]
hboutemy opened a new pull request, #269:
URL: https://github.com/apache/maven-plugin-tools/pull/269
(no comment)
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MPLUGIN-511 add plugin requirements history detection [maven-plugin-tools]
Posted by "hboutemy (via GitHub)" <gi...@apache.org>.
hboutemy commented on code in PR #269:
URL: https://github.com/apache/maven-plugin-tools/pull/269#discussion_r1522316963
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugin/plugin/report/RequirementsHistory.java:
##########
@@ -72,16 +73,23 @@ public String toString() {
return sb.toString();
}
+ public static RequirementsHistory discoverRequirements(MavenProject project) {
+ RequirementsHistory req = new RequirementsHistory();
+ req.version = project.getVersion();
+ req.jdk = discoverJdkRequirement(project, null);
+ req.maven = discoverMavenRequirement(project, null);
+ return req;
+ }
/**
* Tries to determine the Maven requirement from either the plugin descriptor or (if not set) from the
* Maven prerequisites element in the POM.
*
* @param project not null
- * @param pluginDescriptor the plugin descriptor (not null)
+ * @param pluginDescriptor the plugin descriptor (can be null)
* @return the Maven version or null if not specified
*/
public static String discoverMavenRequirement(MavenProject project, PluginDescriptor pluginDescriptor) {
- if (StringUtils.isNotBlank(pluginDescriptor.getRequiredMavenVersion())) {
+ if (pluginDescriptor != null && StringUtils.isNotBlank(pluginDescriptor.getRequiredMavenVersion())) {
Review Comment:
because it was until now used only with current release, where the descriptor just had been generated
but now it is used with history, and I did not take time to extract generated content from Maven Central: I chose to just add code to not choke on null value
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MPLUGIN-511 add plugin requirements history detection [maven-plugin-tools]
Posted by "hboutemy (via GitHub)" <gi...@apache.org>.
hboutemy commented on code in PR #269:
URL: https://github.com/apache/maven-plugin-tools/pull/269#discussion_r1522314835
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugin/plugin/report/PluginReport.java:
##########
@@ -93,6 +110,9 @@ public class PluginReport extends AbstractMavenReport {
@Parameter
private List<RequirementsHistory> requirementsHistories = new ArrayList<>();
+ @Parameter(defaultValue = "[0,)")
+ private String detectRequirementsHistory;
Review Comment:
yes, it looked strange, but I did not precisely see why :)
renamed to `requirementsHistoryDetectRange`
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MPLUGIN-511 add plugin requirements history detection [maven-plugin-tools]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #269:
URL: https://github.com/apache/maven-plugin-tools/pull/269#discussion_r1521308296
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugin/plugin/report/PluginReport.java:
##########
@@ -93,6 +110,9 @@ public class PluginReport extends AbstractMavenReport {
@Parameter
private List<RequirementsHistory> requirementsHistories = new ArrayList<>();
+ @Parameter(defaultValue = "[0,)")
+ private String detectRequirementsHistory;
Review Comment:
The naming includes to be boolean, but is string. Weird.
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugin/plugin/report/RequirementsHistory.java:
##########
@@ -118,33 +126,32 @@ public static String discoverJdkRequirement(MavenProject project, PluginDescript
}
jdk = getPluginParameter(compiler, "release");
- if (jdk != null) {
- return jdk;
+ if (jdk == null) {
+ jdk = project.getProperties().getProperty("maven.compiler.release");
}
- jdk = project.getProperties().getProperty("maven.compiler.release");
- if (jdk != null) {
- return jdk;
+ if (jdk == null) {
+ jdk = getPluginParameter(compiler, "target");
}
- jdk = getPluginParameter(compiler, "target");
- if (jdk != null) {
- return jdk;
+ if (jdk == null) {
+ // default value
+ jdk = project.getProperties().getProperty("maven.compiler.target");
}
- // default value
- jdk = project.getProperties().getProperty("maven.compiler.target");
- if (jdk != null) {
- return jdk;
- }
+ if (jdk == null) {
+ String version = (compiler == null) ? null : compiler.getVersion();
- String version = (compiler == null) ? null : compiler.getVersion();
-
- if (version != null) {
- return "Default target for maven-compiler-plugin version " + version;
+ if (version != null) {
+ return "Default target for maven-compiler-plugin version " + version;
+ }
+ } else {
+ if (Arrays.asList("1.5", "1.6", "1.7", "1.8").contains(jdk)) {
Review Comment:
One should not be able to write 1.9, 1.10, etc.
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugin/plugin/report/PluginReport.java:
##########
@@ -150,6 +179,29 @@ protected void executeReport(Locale locale) throws MavenReportException {
// Generate the mojos' documentation
generateMojosDocumentation(pluginDescriptor, locale);
+ if (requirementsHistories.isEmpty()) {
+ // detect requirements history
+ String v = null;
+ try {
+ List<Version> versions = discoverVersions(detectRequirementsHistory);
+ getLog().info("Detecting requirements history for " + detectRequirementsHistory + ": "
+ + versions.size());
+
+ Collections.reverse(versions);
+ for (Version version : versions) {
+ v = version.toString();
+ MavenProject versionProject = buildMavenProject(v);
+ RequirementsHistory requirements = RequirementsHistory.discoverRequirements(versionProject);
+ requirementsHistories.add(requirements);
+ getLog().info("- " + requirements);
Review Comment:
This should be indented
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugin/plugin/report/RequirementsHistory.java:
##########
@@ -72,16 +73,23 @@ public String toString() {
return sb.toString();
}
+ public static RequirementsHistory discoverRequirements(MavenProject project) {
+ RequirementsHistory req = new RequirementsHistory();
+ req.version = project.getVersion();
+ req.jdk = discoverJdkRequirement(project, null);
+ req.maven = discoverMavenRequirement(project, null);
+ return req;
+ }
/**
* Tries to determine the Maven requirement from either the plugin descriptor or (if not set) from the
* Maven prerequisites element in the POM.
*
* @param project not null
- * @param pluginDescriptor the plugin descriptor (not null)
+ * @param pluginDescriptor the plugin descriptor (can be null)
* @return the Maven version or null if not specified
*/
public static String discoverMavenRequirement(MavenProject project, PluginDescriptor pluginDescriptor) {
- if (StringUtils.isNotBlank(pluginDescriptor.getRequiredMavenVersion())) {
+ if (pluginDescriptor != null && StringUtils.isNotBlank(pluginDescriptor.getRequiredMavenVersion())) {
Review Comment:
How can it be suddenly `null`?
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MPLUGIN-511 add plugin requirements history detection [maven-plugin-tools]
Posted by "hboutemy (via GitHub)" <gi...@apache.org>.
hboutemy commented on code in PR #269:
URL: https://github.com/apache/maven-plugin-tools/pull/269#discussion_r1524374220
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugin/plugin/report/PluginReport.java:
##########
@@ -93,6 +110,14 @@ public class PluginReport extends AbstractMavenReport {
@Parameter
private List<RequirementsHistory> requirementsHistories = new ArrayList<>();
+ /**
+ * Plugin's version range for automatic detection of requirements history.
+ *
+ * @since 3.12.0
+ */
+ @Parameter(defaultValue = "[0,)")
+ private String requirementsHistoryDetectRange;
Review Comment:
done
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MPLUGIN-511 add plugin requirements history detection [maven-plugin-tools]
Posted by "hboutemy (via GitHub)" <gi...@apache.org>.
hboutemy commented on code in PR #269:
URL: https://github.com/apache/maven-plugin-tools/pull/269#discussion_r1522314214
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugin/plugin/report/PluginReport.java:
##########
@@ -93,6 +110,9 @@ public class PluginReport extends AbstractMavenReport {
@Parameter
private List<RequirementsHistory> requirementsHistories = new ArrayList<>();
+ @Parameter(defaultValue = "[0,)")
+ private String detectRequirementsHistory;
Review Comment:
sure, added
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MPLUGIN-511 add plugin requirements history detection [maven-plugin-tools]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #269:
URL: https://github.com/apache/maven-plugin-tools/pull/269#discussion_r1514029558
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugin/plugin/report/PluginReport.java:
##########
@@ -93,6 +110,9 @@ public class PluginReport extends AbstractMavenReport {
@Parameter
private List<RequirementsHistory> requirementsHistories = new ArrayList<>();
+ @Parameter(defaultValue = "[0,)")
+ private String detectRequirementsHistory;
Review Comment:
A documentation will be appreciated
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugin/plugin/report/RequirementsHistory.java:
##########
@@ -118,33 +126,32 @@ public static String discoverJdkRequirement(MavenProject project, PluginDescript
}
jdk = getPluginParameter(compiler, "release");
- if (jdk != null) {
- return jdk;
+ if (jdk == null) {
+ jdk = project.getProperties().getProperty("maven.compiler.release");
}
- jdk = project.getProperties().getProperty("maven.compiler.release");
- if (jdk != null) {
- return jdk;
+ if (jdk == null) {
+ jdk = getPluginParameter(compiler, "target");
}
- jdk = getPluginParameter(compiler, "target");
- if (jdk != null) {
- return jdk;
+ if (jdk == null) {
+ // default value
+ jdk = project.getProperties().getProperty("maven.compiler.target");
}
- // default value
- jdk = project.getProperties().getProperty("maven.compiler.target");
- if (jdk != null) {
- return jdk;
- }
+ if (jdk == null) {
+ String version = (compiler == null) ? null : compiler.getVersion();
- String version = (compiler == null) ? null : compiler.getVersion();
-
- if (version != null) {
- return "Default target for maven-compiler-plugin version " + version;
+ if (version != null) {
+ return "Default target for maven-compiler-plugin version " + version;
+ }
+ } else {
+ if (Arrays.asList("1.5", "1.6", "1.7", "1.8").contains(jdk)) {
Review Comment:
`startsWith("1.")` ?
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MPLUGIN-511 add plugin requirements history detection [maven-plugin-tools]
Posted by "hboutemy (via GitHub)" <gi...@apache.org>.
hboutemy merged PR #269:
URL: https://github.com/apache/maven-plugin-tools/pull/269
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MPLUGIN-511 add plugin requirements history detection [maven-plugin-tools]
Posted by "hboutemy (via GitHub)" <gi...@apache.org>.
hboutemy commented on code in PR #269:
URL: https://github.com/apache/maven-plugin-tools/pull/269#discussion_r1522310376
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugin/plugin/report/RequirementsHistory.java:
##########
@@ -118,33 +126,32 @@ public static String discoverJdkRequirement(MavenProject project, PluginDescript
}
jdk = getPluginParameter(compiler, "release");
- if (jdk != null) {
- return jdk;
+ if (jdk == null) {
+ jdk = project.getProperties().getProperty("maven.compiler.release");
}
- jdk = project.getProperties().getProperty("maven.compiler.release");
- if (jdk != null) {
- return jdk;
+ if (jdk == null) {
+ jdk = getPluginParameter(compiler, "target");
}
- jdk = getPluginParameter(compiler, "target");
- if (jdk != null) {
- return jdk;
+ if (jdk == null) {
+ // default value
+ jdk = project.getProperties().getProperty("maven.compiler.target");
}
- // default value
- jdk = project.getProperties().getProperty("maven.compiler.target");
- if (jdk != null) {
- return jdk;
- }
+ if (jdk == null) {
+ String version = (compiler == null) ? null : compiler.getVersion();
- String version = (compiler == null) ? null : compiler.getVersion();
-
- if (version != null) {
- return "Default target for maven-compiler-plugin version " + version;
+ if (version != null) {
+ return "Default target for maven-compiler-plugin version " + version;
+ }
+ } else {
+ if (Arrays.asList("1.5", "1.6", "1.7", "1.8").contains(jdk)) {
Review Comment:
and 1.4 is 1.4: only starting with 1.5 we have the 5 alias
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MPLUGIN-511 add plugin requirements history detection [maven-plugin-tools]
Posted by "hboutemy (via GitHub)" <gi...@apache.org>.
hboutemy commented on code in PR #269:
URL: https://github.com/apache/maven-plugin-tools/pull/269#discussion_r1522315605
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugin/plugin/report/PluginReport.java:
##########
@@ -150,6 +179,29 @@ protected void executeReport(Locale locale) throws MavenReportException {
// Generate the mojos' documentation
generateMojosDocumentation(pluginDescriptor, locale);
+ if (requirementsHistories.isEmpty()) {
+ // detect requirements history
+ String v = null;
+ try {
+ List<Version> versions = discoverVersions(detectRequirementsHistory);
+ getLog().info("Detecting requirements history for " + detectRequirementsHistory + ": "
+ + versions.size());
+
+ Collections.reverse(versions);
+ for (Version version : versions) {
+ v = version.toString();
+ MavenProject versionProject = buildMavenProject(v);
+ RequirementsHistory requirements = RequirementsHistory.discoverRequirements(versionProject);
+ requirementsHistories.add(requirements);
+ getLog().info("- " + requirements);
Review Comment:
if you prefer: added 2 spaces
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MPLUGIN-511 add plugin requirements history detection [maven-plugin-tools]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #269:
URL: https://github.com/apache/maven-plugin-tools/pull/269#discussion_r1523907398
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugin/plugin/report/PluginReport.java:
##########
@@ -93,6 +110,14 @@ public class PluginReport extends AbstractMavenReport {
@Parameter
private List<RequirementsHistory> requirementsHistories = new ArrayList<>();
+ /**
+ * Plugin's version range for automatic detection of requirements history.
+ *
+ * @since 3.12.0
+ */
+ @Parameter(defaultValue = "[0,)")
+ private String requirementsHistoryDetectRange;
Review Comment:
`requirementsHistoryDetectionRange`
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugin/plugin/report/RequirementsHistory.java:
##########
@@ -72,16 +73,23 @@ public String toString() {
return sb.toString();
}
+ public static RequirementsHistory discoverRequirements(MavenProject project) {
+ RequirementsHistory req = new RequirementsHistory();
+ req.version = project.getVersion();
+ req.jdk = discoverJdkRequirement(project, null);
+ req.maven = discoverMavenRequirement(project, null);
+ return req;
+ }
/**
* Tries to determine the Maven requirement from either the plugin descriptor or (if not set) from the
* Maven prerequisites element in the POM.
*
* @param project not null
- * @param pluginDescriptor the plugin descriptor (not null)
+ * @param pluginDescriptor the plugin descriptor (can be null)
* @return the Maven version or null if not specified
*/
public static String discoverMavenRequirement(MavenProject project, PluginDescriptor pluginDescriptor) {
- if (StringUtils.isNotBlank(pluginDescriptor.getRequiredMavenVersion())) {
+ if (pluginDescriptor != null && StringUtils.isNotBlank(pluginDescriptor.getRequiredMavenVersion())) {
Review Comment:
I see, makes sense.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org