You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "mbien (via GitHub)" <gi...@apache.org> on 2025/02/17 20:36:49 UTC
[PR] Remove guava dependency v2 [netbeans-mavenutils-nbm-maven-plugin]
mbien opened a new pull request, #220:
URL: https://github.com/apache/netbeans-mavenutils-nbm-maven-plugin/pull/220
(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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Remove guava dependency v2 [netbeans-mavenutils-nbm-maven-plugin]
Posted by "ebarboni (via GitHub)" <gi...@apache.org>.
ebarboni merged PR #220:
URL: https://github.com/apache/netbeans-mavenutils-nbm-maven-plugin/pull/220
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Remove guava dependency v2 [netbeans-mavenutils-nbm-maven-plugin]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #220:
URL: https://github.com/apache/netbeans-mavenutils-nbm-maven-plugin/pull/220#discussion_r1960219763
##########
nbm-maven-plugin/src/main/java/org/apache/netbeans/nbm/CreateClusterAppMojo.java:
##########
@@ -1091,6 +1086,12 @@ static Map<String, Set<String>> computeClusterOrdering(Map<String, Set<String>>
return cluster2depClusters;
}
+ private static Set<String> intersection(Set<String> first, Collection<String> second) {
+ HashSet<String> intersection = new HashSet<>(first);
Review Comment:
Sorry for the noise. I checked against the original spec without paying enough attention to how the result was actually being used!
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Remove guava dependency v2 [netbeans-mavenutils-nbm-maven-plugin]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #220:
URL: https://github.com/apache/netbeans-mavenutils-nbm-maven-plugin/pull/220#discussion_r1960209262
##########
nbm-maven-plugin/src/main/java/org/apache/netbeans/nbm/CreateClusterAppMojo.java:
##########
@@ -1091,6 +1086,12 @@ static Map<String, Set<String>> computeClusterOrdering(Map<String, Set<String>>
return cluster2depClusters;
}
+ private static Set<String> intersection(Set<String> first, Collection<String> second) {
+ HashSet<String> intersection = new HashSet<>(first);
Review Comment:
not sure I understand the concern. The set is only used in the form of `intersection.isEmpty()`.
Maybe I should change the method to `doesIntersect()` and the impl to `return new HashSet(first).retainAll(second)`?
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Remove guava dependency v2 [netbeans-mavenutils-nbm-maven-plugin]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #220:
URL: https://github.com/apache/netbeans-mavenutils-nbm-maven-plugin/pull/220#discussion_r1960306834
##########
nbm-maven-plugin/src/main/java/org/apache/netbeans/nbm/CreateClusterAppMojo.java:
##########
@@ -1091,6 +1086,12 @@ static Map<String, Set<String>> computeClusterOrdering(Map<String, Set<String>>
return cluster2depClusters;
}
+ private static Set<String> intersection(Set<String> first, Collection<String> second) {
+ HashSet<String> intersection = new HashSet<>(first);
Review Comment:
actually, let me change the code a bit. Some of the if checks can be changed to `col.contains(other)`
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Remove guava dependency v2 [netbeans-mavenutils-nbm-maven-plugin]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #220:
URL: https://github.com/apache/netbeans-mavenutils-nbm-maven-plugin/pull/220#discussion_r1960209262
##########
nbm-maven-plugin/src/main/java/org/apache/netbeans/nbm/CreateClusterAppMojo.java:
##########
@@ -1091,6 +1086,12 @@ static Map<String, Set<String>> computeClusterOrdering(Map<String, Set<String>>
return cluster2depClusters;
}
+ private static Set<String> intersection(Set<String> first, Collection<String> second) {
+ HashSet<String> intersection = new HashSet<>(first);
Review Comment:
not sure I understand the concern. The set is only used in the form of `intersection.isEmpty()`.
~Maybe I should change the method to `doesIntersect()` and the impl to `return new HashSet(first).retainAll(second)`?~
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Remove guava dependency v2 [netbeans-mavenutils-nbm-maven-plugin]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #220:
URL: https://github.com/apache/netbeans-mavenutils-nbm-maven-plugin/pull/220#discussion_r1960217576
##########
nbm-maven-plugin/src/main/java/org/apache/netbeans/nbm/CreateClusterAppMojo.java:
##########
@@ -1091,6 +1086,12 @@ static Map<String, Set<String>> computeClusterOrdering(Map<String, Set<String>>
return cluster2depClusters;
}
+ private static Set<String> intersection(Set<String> first, Collection<String> second) {
+ HashSet<String> intersection = new HashSet<>(first);
Review Comment:
please ignore the second sentence, the boolean return is a modification marker not an indicator for intersection.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Remove guava dependency v2 [netbeans-mavenutils-nbm-maven-plugin]
Posted by "ebarboni (via GitHub)" <gi...@apache.org>.
ebarboni commented on PR #220:
URL: https://github.com/apache/netbeans-mavenutils-nbm-maven-plugin/pull/220#issuecomment-2678430275
LGTM, juste need a rebase
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Remove guava dependency v2 [netbeans-mavenutils-nbm-maven-plugin]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #220:
URL: https://github.com/apache/netbeans-mavenutils-nbm-maven-plugin/pull/220#discussion_r1959518460
##########
nbm-maven-plugin/src/main/java/org/apache/netbeans/nbm/CreateClusterAppMojo.java:
##########
@@ -1091,6 +1086,12 @@ static Map<String, Set<String>> computeClusterOrdering(Map<String, Set<String>>
return cluster2depClusters;
}
+ private static Set<String> intersection(Set<String> first, Collection<String> second) {
+ HashSet<String> intersection = new HashSet<>(first);
Review Comment:
Are you 100% sure that none of the sets passed in here are ordered? Replaced code states it maintains iteration order of `first`.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists