You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2022/09/23 03:55:02 UTC

[GitHub] [netbeans] mbien opened a new pull request, #4678: Improve java platform selection logic for hints.

mbien opened a new pull request, #4678:
URL: https://github.com/apache/netbeans/pull/4678

   followup to #4672. 
   
    - old logic didn't check installed platforms if the spec version
      of the default platform was null
    - cap at feature version of nb-javac (or javac, if uninstalled)
    - default platform is the fallback
    - converted logic to streams
   
   This code runs when code inspections are manually run (e.g. refactor -> inspect and transform). The editor is probably just using the default platform?
   
   I am actually not sure why this selection logic is needed at all - but this should make it less buggy at least.


-- 
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


[GitHub] [netbeans] neilcsmith-net commented on pull request #4678: Improve java platform selection logic for hints.

Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on PR #4678:
URL: https://github.com/apache/netbeans/pull/4678#issuecomment-1276274823

   > isValid is very basic unfortunately:
   
   Yes, I saw.
   
   > Should we move the spec version check into that method?
   
   Probably not at this point?  But could in mind better validation of platforms for NB17?


-- 
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


[GitHub] [netbeans] neilcsmith-net commented on pull request #4678: Improve java platform selection logic for hints.

Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on PR #4678:
URL: https://github.com/apache/netbeans/pull/4678#issuecomment-1264612666

   Looks OK at a glance.  I'm curious whether a platform that passes `isValid` should ever return a `null` spec version?


-- 
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


[GitHub] [netbeans] mbien commented on pull request #4678: Improve java platform selection logic for hints.

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4678:
URL: https://github.com/apache/netbeans/pull/4678#issuecomment-1276153516

   > Looks OK at a glance. I'm curious whether a platform that passes `isValid` should ever return a `null` spec version?
   
   @neilcsmith-net `isValid` is very basic unfortunately:
   ```
       public boolean isValid() {
           return !getInstallFolders().isEmpty();
       }
   ```
   Should we move the speck version check into that method?
   I am a bit afraid to move more checks there because who knows what else is affected by that. E.g what if NB thinks the default platform isn't valid on some exotic setups. 


-- 
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


[GitHub] [netbeans] mbien merged pull request #4678: Improve java platform selection logic for hints.

Posted by GitBox <gi...@apache.org>.
mbien merged PR #4678:
URL: https://github.com/apache/netbeans/pull/4678


-- 
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


[GitHub] [netbeans] neilcsmith-net commented on a diff in pull request #4678: Improve java platform selection logic for hints.

Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on code in PR #4678:
URL: https://github.com/apache/netbeans/pull/4678#discussion_r985219586


##########
java/spi.java.hints/src/org/netbeans/modules/java/hints/spiimpl/Utilities.java:
##########
@@ -1090,7 +1091,7 @@ public synchronized ClasspathInfo createUniversalCPInfo() {
                     .filter((p) -> "j2se".equals(p.getSpecification().getName()))
                     .filter((p) -> p.getSpecification().getVersion() != null)
                     .filter((p) -> p.getSpecification().getVersion().compareTo(maxVersion) < 0)
-                    .max((p1, p2) -> p1.getSpecification().getVersion().compareTo(p2.getSpecification().getVersion()))
+                    .max(Comparator.comparing((p) -> p.getSpecification().getVersion()))

Review Comment:
   @mbien I _think_ that's fine.  And leads to nicer code here.  Still binary compatible because of bridge method not picked up by sigtest?  @jtulach the best person to check with on sigtests and compatible API updates.



-- 
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


[GitHub] [netbeans] mbien commented on a diff in pull request #4678: Improve java platform selection logic for hints.

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4678:
URL: https://github.com/apache/netbeans/pull/4678#discussion_r985175283


##########
java/spi.java.hints/src/org/netbeans/modules/java/hints/spiimpl/Utilities.java:
##########
@@ -1090,7 +1091,7 @@ public synchronized ClasspathInfo createUniversalCPInfo() {
                     .filter((p) -> "j2se".equals(p.getSpecification().getName()))
                     .filter((p) -> p.getSpecification().getVersion() != null)
                     .filter((p) -> p.getSpecification().getVersion().compareTo(maxVersion) < 0)
-                    .max((p1, p2) -> p1.getSpecification().getVersion().compareTo(p2.getSpecification().getVersion()))
+                    .max(Comparator.comparing((p) -> p.getSpecification().getVersion()))

Review Comment:
   to do this I had add generics to `SpecificationVersion`'s `Comparable` interface.
   
   i believe this should be safe since the class is final and the `compareTo` method would have thrown a `ClassCastException` anyway when anything other than SpecificationVersion would be passed as parameter.
   
   This requires a sig file change. 
   
   @neilcsmith-net could you take a look at the second commit if that is ok? The second commit is entirely optional, no big deal to drop it. I didn't bump any versions.



-- 
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