On Fri, 25 Sep 2020 21:45:39 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
> We have a number of missing javadoc tags and comments in the desktop module. > Most of the missing comments are related to the serialized form. > > The fix: > - Adds missing comments to the non-static/non-transient fields(even > private) of the "serializable" classes > - Adds comments to the "serializable" classes even if those classes are > non-public > - Fixes references/adds missing tags to the special methods(like > readObject/writeObject) > - Delete the java.awt.PeerFixer class. > > I need advice about what exact change should be reviewed in the CSR(except > PeerFixer removal) > > Note that in some cases I added the comments to the "implementation details", > so I did not specify it fully. > > The old review request: > https://mail.openjdk.java.net/pipermail/beans-dev/2020-August/000423.html I think the main thing here is I would separate out removing the duplicate PeerFixer into a new bug. I also see that the CSR is still just a pure template. src/java.desktop/share/classes/java/awt/ScrollPane.java line 815: > 813: > 814: /* > 815: * In JDK 1.1.1, the pkg private class java.awt.PeerFixer was moved to As I mentioned in the OLD review thread for hg, this definitely needs a CSR and it needs to be called out. I think it should be separated out from this fix which is about fixing doclint warnings but here you are making an incompatible change. Let's not mix the two. src/java.desktop/share/classes/java/awt/CheckboxMenuItem.java line 434: > 432: * @serial > 433: */ > 434: private int checkboxMenuItemSerializedDataVersion = 1; OK. Good this was being discussed in the old review and it should stay. src/java.desktop/share/classes/java/awt/ContainerOrderFocusTraversalPolicy.java line 75: > 73: * This constant is used when the backward focus traversal order is > active. > 74: */ > 75: private final int BACKWARD_TRAVERSAL = 1; I see you also reverted the change of these two to static so that is also good. ------------- Changes requested by prr (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/369
