dtrebbien commented on issue #361: my first refactoring session URL: https://github.com/apache/incubator-netbeans/pull/361#issuecomment-357448838 I think 22e386e963093194eb0a2c8bdf57c5544d86d691 needs to be reverted, or please explain why you deleted those unit tests. Also, I think 1ce564b3a65ae3cdcb6f6ba0abd638b68c847751 is starting to go overboard. Some thoughts: * Changing ```java fireProgressListenerStart(0, COMMIT_STEPS + commits.size() * COMMIT_STEPS + 1); ``` to: ```java { int count = COMMIT_STEPS + commits.size() * COMMIT_STEPS + 1; fireProgressListenerStart(0, count); } ``` and the other similar changes are unnecessary, in my opinion. I like the introduction of a variable for the result of DataObject.getRegistry().getModified() because it was unclear that this has type `DataObject[]` instead of some type of `Iterable`; but, other introduced variables seem unnecessary. * I don't think that reallyDoRefactoring() and reallyUndoRefactoring() should be inlined. Don't get me wrong, I appreciate the reason why you did this. However, I think that the original pattern should be kept, as it is a perfectly acceptable pattern to use. * `Arrays.asList(...).stream()` can be replaced with `Stream.of(...)`. * With the exception of the JavaDoc comment on performChange(), I think that the other new comments are unnecessary and should be removed. One final thought: I think that commit messages should not be cutesy or contain jokes (e.g. "against the order of maw") because the developers working on this project come from different backgrounds and may not get all of the references. I had to search "order of maw" to figure out that it is a Warcraft reference (I think? still not sure...).
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- 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
