You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/10/10 22:00:22 UTC
[GitHub] [maven] elharo commented on a diff in pull request #819: [MNG-7553] Restore compatibility on SettingsBuilder, ToolchainsBuilder and MojoDescriptorCreator
elharo commented on code in PR #819:
URL: https://github.com/apache/maven/pull/819#discussion_r991663543
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Session.java:
##########
@@ -56,9 +56,21 @@
@Nonnull
SessionData getData();
+ /**
+ * Gets the user properties to use for interpolation. The user properties have been configured directly by the user
+ * on his discretion, e.g. via the {@code -Dkey=value} parameter on the command line.
Review Comment:
delete "on his discretion"
, --> ;
##########
api/maven-api-core/src/main/java/org/apache/maven/api/services/SettingsBuilder.java:
##########
@@ -0,0 +1,71 @@
+package org.apache.maven.api.services;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.nio.file.Path;
+
+import org.apache.maven.api.Service;
+import org.apache.maven.api.Session;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Builds the effective settings from a user settings file and/or a global settings file.
Review Comment:
what is "effective settings"? How does that differ from other settings?
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Session.java:
##########
@@ -56,9 +56,21 @@
@Nonnull
SessionData getData();
+ /**
+ * Gets the user properties to use for interpolation. The user properties have been configured directly by the user
+ * on his discretion, e.g. via the {@code -Dkey=value} parameter on the command line.
+ *
+ * @return The user properties, never {@code null}.
+ */
@Nonnull
Map<String, String> getUserProperties();
+ /**
+ * Gets the system properties to use for interpolation. The system properties are collected from the runtime
+ * environment like {@link System#getProperties()} and environment variables.
Review Comment:
like --> such as
##########
api/maven-api-core/src/main/java/org/apache/maven/api/services/SettingsBuilder.java:
##########
@@ -0,0 +1,71 @@
+package org.apache.maven.api.services;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.nio.file.Path;
+
+import org.apache.maven.api.Service;
+import org.apache.maven.api.Session;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Builds the effective settings from a user settings file and/or a global settings file.
+ */
+public interface SettingsBuilder extends Service
+{
+
+ /**
+ * Builds the effective settings of the specified settings files.
+ *
+ * @param request The settings building request that holds the parameters, must not be {@code null}.
Review Comment:
The --> the (per Oracle, API doc params do not begin with a capital letter
##########
api/maven-api-core/src/main/java/org/apache/maven/api/services/BuilderProblemSeverity.java:
##########
@@ -27,7 +27,7 @@
* @since 4.0
*/
@Experimental
-public enum ProjectBuilderProblemSeverity
+public enum BuilderProblemSeverity
Review Comment:
rename seems unnecessary
##########
api/maven-api-core/src/main/java/org/apache/maven/api/services/ProjectBuilder.java:
##########
@@ -51,13 +51,13 @@ public interface ProjectBuilder extends Service
* Creates a {@link org.apache.maven.api.Project} from a POM file.
*
* @param session The {@link Session}, must not be {@code null}.
- * @param source The {@link ProjectBuilderSource}, must not be {@code null}.
- * @throws ProjectBuilderException if the project cannot be created
+ * @param source The {@link Source}, must not be {@code null}.
+ * @throws ProjectBuilderException if the project can not be created
Review Comment:
revert, cannot is one word
##########
api/maven-api-core/src/main/java/org/apache/maven/api/services/ProjectBuilder.java:
##########
@@ -51,13 +51,13 @@ public interface ProjectBuilder extends Service
* Creates a {@link org.apache.maven.api.Project} from a POM file.
*
* @param session The {@link Session}, must not be {@code null}.
- * @param source The {@link ProjectBuilderSource}, must not be {@code null}.
- * @throws ProjectBuilderException if the project cannot be created
+ * @param source The {@link Source}, must not be {@code null}.
Review Comment:
The --> the
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Session.java:
##########
@@ -56,9 +56,21 @@
@Nonnull
SessionData getData();
+ /**
Review Comment:
Javadoc additions to existing API are useful but can be included in a separate PR that is easier to approve.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/services/SettingsBuilderException.java:
##########
@@ -0,0 +1,43 @@
+package org.apache.maven.api.services;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.api.annotations.Experimental;
+
+/**
+ * The Exception class throw by the {@link SettingsBuilder}.
+ *
+ * @since 4.0
+ */
+@Experimental
+public class SettingsBuilderException
+ extends MavenException
+{
+ /**
+ * @param message The message to give.
+ * @param e The {@link Exception}.
Review Comment:
don't repeat type information in doc comments. It's redundant. Instead explain what this exception is
##########
api/maven-api-core/src/main/java/org/apache/maven/api/services/SettingsBuilder.java:
##########
@@ -0,0 +1,71 @@
+package org.apache.maven.api.services;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.nio.file.Path;
+
+import org.apache.maven.api.Service;
+import org.apache.maven.api.Session;
+import org.apache.maven.api.annotations.Nonnull;
+
+/**
+ * Builds the effective settings from a user settings file and/or a global settings file.
+ */
+public interface SettingsBuilder extends Service
+{
+
+ /**
+ * Builds the effective settings of the specified settings files.
+ *
+ * @param request The settings building request that holds the parameters, must not be {@code null}.
+ * @return The result of the settings building, never {@code null}.
+ * @throws SettingsBuilderException If the effective settings could not be built.
Review Comment:
if the effective settings could not be built
--
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