eirikbakke commented on pull request #2119: URL: https://github.com/apache/netbeans/pull/2119#issuecomment-629856344
Thank you for this! Main concerns: 1) Is it really necessary to use a module installer? If so, it should not run unless the functionality is actually enabled (is this the case?). 2) There are various listeners attached to global objects (Preferences, at least), which can lead to memory leaks. There are some (I think) incorrect attempts to use weak listeners. This would need to be reworked, ideally by removing listeners explicitly, at the right time, rather than relying on weak listeners. Weak listeners can be used, but then it needs to be very clear when the objects are really going to be garbage collected, and why they won't be garbage collected ideally. 3) This patch uses an internationalization library (omegat) that is not standard in the NetBeans codebase. It would be preferable to do internationalization in the same way as other modules. 4) This is a very big patch for relatively simple functionality; I think a lot of auto-generated files were included. Those should be removed and put in a .gitignore. (Or maybe they will go away if suggestion (3) is implemented.) Also, would you be able to post a screenshot of the AutoSavePanel in the settings pane? Makes it easier to review the UI without building the patch ourselves. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected] For further information about the NetBeans mailing lists, visit: https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
