elharo commented on code in PR #1208:
URL: https://github.com/apache/maven/pull/1208#discussion_r1274768736
##########
maven-compat/src/main/java/org/apache/maven/profiles/activation/OperatingSystemProfileActivator.java:
##########
@@ -18,17 +18,48 @@
*/
package org.apache.maven.profiles.activation;
+import java.util.Locale;
+
import org.apache.maven.model.Activation;
import org.apache.maven.model.ActivationOS;
import org.apache.maven.model.Profile;
-import org.codehaus.plexus.util.Os;
/**
* OperatingSystemProfileActivator
*/
@Deprecated
public class OperatingSystemProfileActivator implements ProfileActivator {
+ public static final String FAMILY_DOS = "dos";
Review Comment:
why are these public? especially in a deprecated class?
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
Review Comment:
These sorts of methods are very tricky with lots of inobvious, platform
dependent edge cases that are hard to get right. I'd be much more comfortable
if we could use a single method from commons-io like FileUtils.copyFile
##########
maven-core/src/main/java/org/apache/maven/utils/introspection/ReflectionValueExtractor.java:
##########
@@ -0,0 +1,325 @@
+/*
+ * 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.
+ */
+package org.apache.maven.utils.introspection;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Array;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * <p>
+ * Using simple dotted expressions to extract the values from an Object
instance, For example we might want to extract a
+ * value like: <code>project.build.sourceDirectory</code>
+ * </p>
+ * <p>
+ * The implementation supports indexed, nested and mapped properties similar
to the JSP way.
+ * </p>
+ *
+ * @author <a href="mailto:[email protected]";>Jason van Zyl </a>
+ * @author <a href="mailto:[email protected]";>Vincent Siveton</a>
+ *
+ * @see <a href=
+ *
"http://struts.apache.org/1.x/struts-taglib/indexedprops.html";>http://struts.apache.org/1.x/struts-taglib/indexedprops.html</a>
+ */
+public class ReflectionValueExtractor {
Review Comment:
non-public if possible
##########
maven-core/src/main/java/org/apache/maven/utils/dag/Vertex.java:
##########
@@ -0,0 +1,151 @@
+/*
Review Comment:
These classes seem to have simply been copied over from plexus. I'm no sure
you can simply remove copyright like that but IANAL
##########
maven-compat/src/main/java/org/apache/maven/repository/legacy/DefaultWagonManager.java:
##########
@@ -465,6 +469,28 @@ public void getRemoteFile(
}
}
+ private void copyFile(File source, File destination) throws IOException {
+ String message;
Review Comment:
declare whrere used, and don't reuse for two different messages
##########
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetector.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * 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.
+ */
+package org.apache.maven.utils.dag;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * @author <a href="[email protected]">Michal Maczka</a>
+ *
+ */
+public class CycleDetector {
+
+ private static final Integer NOT_VISITED = 0;
+
+ private static final Integer VISITING = 1;
+
+ private static final Integer VISITED = 2;
+
+ public static List<String> hasCycle(final DAG graph) {
+ final List<Vertex> vertices = graph.getVertices();
+
+ final Map<Vertex, Integer> vertexStateMap = new HashMap<>();
+
+ List<String> retValue = null;
+
+ for (Vertex vertex : vertices) {
+ if (isNotVisited(vertex, vertexStateMap)) {
+ retValue = introducesCycle(vertex, vertexStateMap);
+
+ if (retValue != null) {
+ break;
+ }
+ }
+ }
+
+ return retValue;
+ }
+
+ /**
+ * This method will be called when an edge leading to given vertex was
added and we want to check if introduction of
Review Comment:
is called
is added
i.e. present tense always in tech writing
##########
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetectedException.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.
+ */
+package org.apache.maven.utils.dag;
+
+import java.util.Iterator;
+import java.util.List;
+
+public class CycleDetectedException extends Exception {
+ private List<String> cycle;
+
+ public CycleDetectedException(final String message, final List<String>
cycle) {
+ super(message);
+
+ this.cycle = cycle;
+ }
+
+ public List<String> getCycle() {
+ return cycle;
+ }
+
+ public String cycleToString() {
+ final StringBuilder buffer = new StringBuilder();
+
+ for (Iterator<String> iterator = cycle.iterator(); iterator.hasNext();
) {
Review Comment:
StringUtils.join from commons-lang?
##########
maven-embedder/src/main/java/org/apache/maven/cli/CLIReportingUtils.java:
##########
@@ -49,6 +50,36 @@ public final class CLIReportingUtils {
public static final String BUILD_VERSION_PROPERTY = "version";
+ public static final String FAMILY_DOS = "dos";
Review Comment:
non-public
##########
maven-core/src/main/java/org/apache/maven/utils/dag/CycleDetector.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * 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.
+ */
+package org.apache.maven.utils.dag;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * @author <a href="[email protected]">Michal Maczka</a>
+ *
+ */
+public class CycleDetector {
+
+ private static final Integer NOT_VISITED = 0;
+
+ private static final Integer VISITING = 1;
+
+ private static final Integer VISITED = 2;
+
+ public static List<String> hasCycle(final DAG graph) {
+ final List<Vertex> vertices = graph.getVertices();
+
+ final Map<Vertex, Integer> vertexStateMap = new HashMap<>();
+
+ List<String> retValue = null;
+
+ for (Vertex vertex : vertices) {
+ if (isNotVisited(vertex, vertexStateMap)) {
+ retValue = introducesCycle(vertex, vertexStateMap);
+
+ if (retValue != null) {
+ break;
+ }
+ }
+ }
+
+ return retValue;
+ }
+
+ /**
+ * This method will be called when an edge leading to given vertex was
added and we want to check if introduction of
+ * this edge has not resulted in apparition of cycle in the graph
+ *
+ * @param vertex the vertex
+ * @param vertexStateMap the vertex Map
+ * @return the found cycle
+ */
+ public static List<String> introducesCycle(final Vertex vertex, final
Map<Vertex, Integer> vertexStateMap) {
Review Comment:
does this need to be public?
##########
maven-core/src/main/java/org/apache/maven/utils/dag/DAG.java:
##########
@@ -0,0 +1,239 @@
+/*
+ * 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.
+ */
+package org.apache.maven.utils.dag;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * DAG = Directed Acyclic Graph
+ *
+ * @author <a href="[email protected]">Michal Maczka</a>
+ *
+ * TODO this class should be renamed from DAG to Dag
+ */
+public class DAG implements Cloneable, Serializable {
Review Comment:
does this class really need to be public?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
