You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "sdedic (via GitHub)" <gi...@apache.org> on 2023/03/17 15:51:17 UTC
[GitHub] [netbeans] sdedic opened a new pull request, #5671: Remove dependency of decl. hints on hints UI; UI plugs into hints SPI.
sdedic opened a new pull request, #5671:
URL: https://github.com/apache/netbeans/pull/5671
The original idea was to ditch `java.hints.ui` from the LSP server, as it is Swing-based, but I've realized that `java.hints.declarative` depend on the `ui` directly just because of one call.
I made a SPI to plug hints UI implemented in `java.hints.ui` and (private) API in `spiimpl` that displays the UI - so now `java.hints.declarative` only (implementation-)depends on `spi.java.hints`.
--
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] matthiasblaesing commented on pull request #5671: Remove dependency of decl. hints on hints UI; UI plugs into hints SPI.
Posted by "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
[GitHub] [netbeans] sdedic merged pull request #5671: Remove dependency of decl. hints on hints UI; UI plugs into hints SPI.
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic merged PR #5671:
URL: https://github.com/apache/netbeans/pull/5671
--
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] sdedic commented on pull request #5671: Remove dependency of decl. hints on hints UI; UI plugs into hints SPI.
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on PR #5671:
URL: https://github.com/apache/netbeans/pull/5671#issuecomment-1484057282
> 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).
> 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::openRecatoringUI` - 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 ...
--
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