Re: [OpenJDK 2D-Dev] RFR: 8251123: doclint warnings about missing javadoc tags and comments [v2]

2020-10-03 Thread Phil Race
On Sat, 3 Oct 2020 00:31:48 GMT, Sergey Bylokhov  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
>
> Sergey Bylokhov has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Update ScrollPane.java
>  - Update based on the feedback

Marked as reviewed by prr (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/369


Re: [OpenJDK 2D-Dev] RFR: 8251123: doclint warnings about missing javadoc tags and comments [v2]

2020-10-02 Thread Sergey Bylokhov
On Sat, 3 Oct 2020 03:57:21 GMT, Sergey Bylokhov  wrote:

>>> I need advice about what exact change should be reviewed in the CSR(except 
>>> PeerFixer removal)
>> 
>> Probably almost all of it.
>
> The PeerFixer is extracted to the separate CR:
> https://bugs.openjdk.java.net/browse/JDK-8253965
> https://github.com/openjdk/jdk/pull/493
> 
> Other review comments fixed as well, CSR is in progress.

CSR is updated.

-

PR: https://git.openjdk.java.net/jdk/pull/369


Re: [OpenJDK 2D-Dev] RFR: 8251123: doclint warnings about missing javadoc tags and comments [v2]

2020-10-02 Thread Sergey Bylokhov
On Fri, 2 Oct 2020 19:46:10 GMT, Phil Race  wrote:

>> 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.
>
>> I need advice about what exact change should be reviewed in the CSR(except 
>> PeerFixer removal)
> 
> Probably almost all of it.

The PeerFixer is extracted to the separate CR:
https://bugs.openjdk.java.net/browse/JDK-8253965
https://github.com/openjdk/jdk/pull/493

Other review comments fixed as well, CSR is in progress.

-

PR: https://git.openjdk.java.net/jdk/pull/369


Re: [OpenJDK 2D-Dev] RFR: 8251123: doclint warnings about missing javadoc tags and comments [v2]

2020-10-02 Thread Sergey Bylokhov
> 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

Sergey Bylokhov has updated the pull request incrementally with two additional 
commits since the last revision:

 - Update ScrollPane.java
 - Update based on the feedback

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/369/files
  - new: https://git.openjdk.java.net/jdk/pull/369/files/cfc1df5d..c1d83696

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=369=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=369=00-01

  Stats: 52 lines in 3 files changed: 49 ins; 1 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/369.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/369/head:pull/369

PR: https://git.openjdk.java.net/jdk/pull/369


Re: [OpenJDK 2D-Dev] RFR: 8251123: doclint warnings about missing javadoc tags and comments

2020-10-02 Thread Phil Race
On Fri, 2 Oct 2020 18:35:25 GMT, Phil Race  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.

> I need advice about what exact change should be reviewed in the CSR(except 
> PeerFixer removal)

Probably almost all of it.

-

PR: https://git.openjdk.java.net/jdk/pull/369


Re: [OpenJDK 2D-Dev] RFR: 8251123: doclint warnings about missing javadoc tags and comments

2020-10-02 Thread Phil Race
On Fri, 25 Sep 2020 21:45:39 GMT, Sergey Bylokhov  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


Re: [OpenJDK 2D-Dev] RFR: 8251123: doclint warnings about missing javadoc tags and comments

2020-10-02 Thread Phil Race
On Fri, 25 Sep 2020 21:45:39 GMT, Sergey Bylokhov  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

src/java.desktop/share/classes/javax/imageio/metadata/IIOMetadataNode.java line 
44:

> 42: /**
> 43:  * A {@code IIODOMException} is thrown by the {@code IIOMetadataNode} in
> 44:  * "exceptional" circumstances.

"A" -> "An"

-

PR: https://git.openjdk.java.net/jdk/pull/369


Re: [OpenJDK 2D-Dev] RFR: 8251123: doclint warnings about missing javadoc tags and comments

2020-09-30 Thread Jayathirth D V
On Fri, 25 Sep 2020 21:45:39 GMT, Sergey Bylokhov  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

Marked as reviewed by jdv (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/369


Re: [OpenJDK 2D-Dev] RFR: 8251123: doclint warnings about missing javadoc tags and comments

2020-09-30 Thread Jayathirth D V
On Fri, 25 Sep 2020 21:45:39 GMT, Sergey Bylokhov  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

src/java.desktop/share/classes/java/awt/Window.java line 248:

> 246:  * Focus transfers for the lightweight components request should be 
> made
> 247:  * synchronously.
> 248:  */

More simpler way "Focus transfers should be synchronous for lightweight 
component requests"

-

PR: https://git.openjdk.java.net/jdk/pull/369


Re: [OpenJDK 2D-Dev] RFR: 8251123: doclint warnings about missing javadoc tags and comments

2020-09-28 Thread Sergey Bylokhov
On Mon, 28 Sep 2020 06:30:31 GMT, Jayathirth D V  wrote:

> Inputs from 
> https://mail.openjdk.java.net/pipermail/beans-dev/2020-August/000424.html are 
> incorporated or is this fresh
> git review?

It is a fresh update, it changes from the old review request. I made it less 
"dangerous".

-

PR: https://git.openjdk.java.net/jdk/pull/369


Re: [OpenJDK 2D-Dev] RFR: 8251123: doclint warnings about missing javadoc tags and comments

2020-09-28 Thread Jayathirth D V
On Fri, 25 Sep 2020 21:45:39 GMT, Sergey Bylokhov  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

Inputs from 
https://mail.openjdk.java.net/pipermail/beans-dev/2020-August/000424.html are 
incorporated or is this fresh
git review?

-

PR: https://git.openjdk.java.net/jdk/pull/369


[OpenJDK 2D-Dev] RFR: 8251123: doclint warnings about missing javadoc tags and comments

2020-09-25 Thread Sergey Bylokhov
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

-

Commit messages:
 - 8251123: doclint warnings about missing javadoc tags and comments

Changes: https://git.openjdk.java.net/jdk/pull/369/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=369=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8251123
  Stats: 1306 lines in 69 files changed: 810 ins; 214 del; 282 mod
  Patch: https://git.openjdk.java.net/jdk/pull/369.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/369/head:pull/369

PR: https://git.openjdk.java.net/jdk/pull/369


[OpenJDK 2D-Dev] RFR: 8251123 doclint warnings about missing javadoc tags and comments

2020-08-09 Thread Sergey Bylokhov

Hello.
Please review the fix for jdk/client.

Bug: https://bugs.openjdk.java.net/browse/JDK-8251123
Fix: http://cr.openjdk.java.net/~serb/8251123/webrev.01

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)

Also, I made additional changes:
 - In one class to suppress the warning I have made two constants static:
   
http://cr.openjdk.java.net/~serb/8251123/webrev.01/src/java.desktop/share/classes/java/awt/ContainerOrderFocusTraversalPolicy.java.sdiff.html
 - Adds @Deprecated(forRemoval = true) to the PeerFixer class:
   
http://cr.openjdk.java.net/~serb/8251123/webrev.01/src/java.desktop/share/classes/java/awt/ScrollPane.java.sdiff.html
   I think we should delete this class, so to not missing this in future I added
   this tag, or could I delete them right now?

I also would like to clarify the status of unused fields, like:
src/java.desktop/share/classes/java/awt/CheckboxMenuItem.java:433: warning: no 
comment
private int checkboxMenuItemSerializedDataVersion = 1;
Could I add "@Deprecated(forRemoval = true)" to them as well, or could I delete 
them right now?


Note that in some cases I added the comments to the "implementation details", 
so I did not specify it fully.

--
Best regards, Sergey.