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/10 18:40:18 UTC

[GitHub] [netbeans] mbastian opened a new pull request, #4619: Extend libs.flatlaf public packages

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

   Extension of the **public packages** for module **libs.flatlaf**.
   
   This is needed for users who want to add [FlatLaf's SwingX addon](https://github.com/JFormDesigner/FlatLaf/tree/main/flatlaf-swingx). This addon has dependencies to two packages that are not part of the public packages.
   
   When trying to use this addon - in combination to a module dependency to libs.flatlaf - one currently receives the following error:
   ```
   Private classes referenced in module: [com.formdev.flatlaf.ui.MigLayoutVisualPadding, 
   com.formdev.flatlaf.icons.FlatAbstractIcon, com.formdev.flatlaf.ui.FlatRoundBorder, com.formdev.flatlaf.ui.FlatArrowButton, 
   com.formdev.flatlaf.ui.FlatUIUtils$RepaintFocusListener, com.formdev.flatlaf.ui.FlatEmptyBorder, 
   com.formdev.flatlaf.ui.FlatButtonUI, com.formdev.flatlaf.ui.FlatUIUtils, com.formdev.flatlaf.ui.FlatLineBorder]
   ```
   
   Adding new public packages should fix this issue.
   
   ---
   **^Add meaningful description above**
   
   By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -
   
    - are all your own work, and you have the right to contribute them.
    - are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).
   
   Please make sure (eg. `git log`) that all commits have a valid name and email address for you in the Author field.
   
   If you're a first time contributor, see the Contributing guidelines for more information.
   
   


-- 
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] DevCharly commented on pull request #4619: Extend libs.flatlaf public packages

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

   Usage of `<subpackages>` seems to change sigtest behavior:
   
   When I run Ant target `gen-sigtest` without `<subpackages>` (commit d739624220de5813b921f55916372f5f62573975), then `org-netbeans-libs-flatlaf.sig` **grows** from 62k to **380k** (because of added packages) and gen-sigtest reports following error:
   ~~~
   RETURN TYPE FOR FlatFileChooserUI::createDirectoryComboBoxRenderer
       CHANGED FROM javax.swing.plaf.metal.MetalFileChooserUI$DirectoryComboBoxRenderer
       TO javax.swing.DefaultListCellRenderer
   ~~~
   
   But when using `<subpackages>` (commit 3c14ba953043549a871ee5054cb54f377e7f9cae), `org-netbeans-libs-flatlaf.sig` **shrinks** from 62k to **24k** and gen-sigtest does not report any error. `org-netbeans-libs-flatlaf.sig` **does not** contain any class from the subpackages.
   
   When using following, `org-netbeans-libs-flatlaf.sig` stays unchanged.
   ~~~xml
   <subpackages>com.formdev.flatlaf</subpackages>
   <package>com.formdev.flatlaf.util</package>
   ~~~
   
   I wonder what the consequences are for module versioning and API checks...
   Does this mean that only package `com.formdev.flatlaf` is checked for API changes?
   And API changes in subpackages are ok?
   
   
   ### Regarding FlatLaf API compatibility
   
   Only classes in packages `com.formdev.flatlaf` and `com.formdev.flatlaf.util` are kept API compatible.
   There is a sigtest task in [flatlaf-core/build.gradle.kts](https://github.com/JFormDesigner/FlatLaf/blob/main/flatlaf-core/build.gradle.kts) that automatically checks it.
   Due to future development, classes in all other package are internal and may (and do) change (without notice).
   
   ### NetBeans module versioning
   
   To my understanding, when doing incompatible API changes, the modules "Major release version" must be increased, which makes all modules, that depend on it, incompatible. IMHO this should be prevented in any case because this causes problems with 3rd party plugins.
   
   E.g. [JFormDesigner v8](https://www.formdev.com/jformdesigner/) depends on module `org.netbeans.libs.flatlaf/1`. If NB 16 would use a newer FlatLaf version with changes in subpackages, it needs to change major version to `org.netbeans.libs.flatlaf/2`, which makes JFormDesigner v8 incompatible with NB16. Sure, can update JFormDesigner to depend on `org.netbeans.libs.flatlaf/2`, but then it no longer works with previous NB versions. So it would be necessary to build and distribute two JFormDesigner builds: one for NB 16 and another one for NB 15 and older. This would be a nightmare for me and for JFormDesigner users...
   
   ### Conclusion
   
   If this PR means that all classes in all FlatLaf subpackages are handled as "module API" and can not changed any more, then I'm against this PR. Then it is better to make changes to `flatlaf-swingx.jar` to avoid usage of non-public packages. (I'll check this)
   
   If sigtest behavior regarding `<subpackages>` is normal and expected (not a bug), then I'm fine with this PR.
   But please also add `<package>com.formdev.flatlaf.util</package>` to `project.xml` to keep  `org-netbeans-libs-flatlaf.sig` unchanged.
   
   Anyway IMO it is strange that `<subpackages>` makes all subpackages public but sigtest ignores them... 😕 


-- 
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] lkishalmi commented on pull request #4619: Extend libs.flatlaf public packages

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

   Well, friend modules are not that welcome anymore. However let me invite @matthiasblaesing into this conversation. He has the wisdom to sort this thing out!


-- 
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 #4619: Extend libs.flatlaf public packages

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

   The problem with implementation versions is, that they create a hard dependency on a _single_ version of the corresponding package. What we could do is hardcode `OpenIDE-Module-Implementation-Version` to the value of the wrapped flatlaf library. That way the communication @DevCharly made about compatibility would be followed exactly, preventing linking against different versions.
   
   Friend module won't work here trivially as `libs.flatlaf`  already exports an API publicly and you can't do both from a single module. We could create a `org.netbeans.libs.flatlaf.full` module, that is part of the netbeans build and reexports the full flatlaf class set to a `org.netbeans.external.flatlaf.swingx` module as a friend API.


-- 
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] lkishalmi commented on a diff in pull request #4619: Extend libs.flatlaf public packages

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


##########
platform/libs.flatlaf/nbproject/project.xml:
##########
@@ -28,6 +28,8 @@
             <public-packages>
                 <package>com.formdev.flatlaf</package>

Review Comment:
   I'd rather go with:
   ```
   <subpackages>com.formdev.flatlaf</subpackages>
   ```
   



-- 
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 #4619: Extend libs.flatlaf public packages

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

   > Well, friend modules are not that welcome anymore.
   
   IMO, this is what friend modules are for and should be considered for (closely related optional modules). It's their use for never stabilised APIs that I personally find unwelcome.


-- 
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] mbastian commented on pull request #4619: Extend libs.flatlaf public packages

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

   Thanks all for the details and clarity on internals!
   
   Based on @DevCharly's recommendation I tried out the implementation dependency solution on my use case. It seems to work well so I guess we can close this PR. For reference, this is what I added to the `nbm-maven-plugin` configuration:
   ```
   <moduleDependencies>
       <dependency>
           <id>org.netbeans.api:org-netbeans-libs-flatlaf</id>
           <type>impl</type>
       </dependency>
   </moduleDependencies>
   ```


-- 
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] mbastian commented on a diff in pull request #4619: Extend libs.flatlaf public packages

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


##########
platform/libs.flatlaf/nbproject/project.xml:
##########
@@ -28,6 +28,8 @@
             <public-packages>
                 <package>com.formdev.flatlaf</package>

Review Comment:
   Thanks, good point I pushed another commit. I actually wasn't aware of this option as I'm more familiar with the Mavenized platform, which is a bit different on this matter.



-- 
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] lkishalmi commented on pull request #4619: Extend libs.flatlaf public packages

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

   Thanks @DevCharly for the deep evaluation. I'm going to look at why subpackages behave unexpected in signature generation.


-- 
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] mbastian closed pull request #4619: Extend libs.flatlaf public packages

Posted by GitBox <gi...@apache.org>.
mbastian closed pull request #4619: Extend libs.flatlaf public packages
URL: https://github.com/apache/netbeans/pull/4619


-- 
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 #4619: Extend libs.flatlaf public packages

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

   @matthiasblaesing ah, yes, good point on the friend module option!  The hardcoded implementation version sounds like it could be the way to do this.


-- 
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] DevCharly commented on pull request #4619: Extend libs.flatlaf public packages

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

   Seem that NetBeans support some kind of "friend modules" that does not have the package restriction.
   It is called **implementation dependency**, which is mentioned here:
   
   - https://netbeans.apache.org/wiki/DevFaqModuleDependencies.asciidoc and
   - https://bits.netbeans.org/dev/javadoc/org-openide-modules/org/openide/modules/doc-files/classpath.html
   
   If I understand this correctly then a implementation dependency depends on a specific version.
   This would be probably good for a FlatLafSwingX module because e.g. `flatlaf-swingx-2.4.jar` should be always used with `flatlaf-2.4.jar`. There is no guarantee that `flatlaf-swingx-2.4.jar` works correctly with older or newer `flatlaf.jar` versions.
   
   @mbastian maybe this is what you're looking for?


-- 
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] DevCharly commented on pull request #4619: Extend libs.flatlaf public packages

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

   Just found a page with more details about implementation dependencies:
   https://netbeans.apache.org/wiki/DevFaqImplementationDependency.asciidoc


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