[GitHub] [netbeans] matthiasblaesing commented on pull request #5671: Remove dependency of decl. hints on hints UI; UI plugs into hints SPI.

["matthiasblaesing (via GitHub)" <gi...@apache.org>]

matthiasblaesing commented on PR #5671:
URL: https://github.com/apache/netbeans/pull/5671#issuecomment-1484122110

   > > Looks sane to me. I don't think this is strictly an API change - the documented change is not part of the exposed API,
   > 
   > I'ts not, but the `spiimpl` package is a semi-api (or this is how it is used by other hint modules). An API should have been created ages ago from the package...
   
   Understood.
   
   > > I also think some methods could have lower visibility (for example `openRefactoringUI`), but as this is not API, I think it is ok.
   > 
   > Please elaborate. Speaking about `org.netbeans.modules.java.hints.spiimpl.Utilities::openRefactoringUI` - the intention was to expose a helper ("api") method that would locate & call the plugged SPI implementation or fail; the method is likely to be called from different packages, hence it should be public ...
   
   Yeah I noticed, that `openRefactoringUI` is only called from `openRefactoringUIOrWarn`, so could be private. But I think it is ok to keep as is, as it might satisfy other usecases where the caller wants to handle missing UI implementations.
   
   So I still think this is ok to merge.


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