<AWT Dev> Request for review: 7027045

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Mon Aug 12 05:32:55 PDT 2013


Hi, Konstantin.
Fix looks fine to me.
Thanks!

On 09.08.2013 14:01, Konstantin Perikov wrote:
> Hi, AWT team
>
> I update fix. Short summary:
>
>   * Javadoc fixed from /* to /** for two serializable fields
>   * Hint about synchronization still exists
>   * Also, I change  <code></code> to {@code}, but left some of them,
>     where <em></em> is using inside <code> tags
>   * Remove "and and" at line 2887
>
>
> Now webrev placed in Dropbox here- 
> https://www.dropbox.com/sh/fz7qws34btuvq8f/KvIB0ZdL79
> You could donwload webrev in one click or open jdk.patch and looks 
> inside, using your favourite browser.
>
> Please review it.
>
> Thanks,
>
> Konstantin.
>
>
>
> 2013/8/8 Konstantin Perikov <konstantin.perikov at gmail.com 
> <mailto:konstantin.perikov at gmail.com>>
>
>     So, what is the best place for webrev, if I don't have access to
>     ftp server?
>     I don't have possibility to set up Apache server for that.
>     Also, maybe I'm wrong, but to view index.html on ftp, you need to
>     download it as well.
>
>     Okay, I will remove "and and" at line 2887.
>
>     About <em> tag, let me look at Javadoc documentation, I have no
>     idea, how it will behave in {@code} tag
>
>     Konstantin.
>
>
>
>
>     2013/8/8 Sergey Bylokhov <Sergey.Bylokhov at oracle.com
>     <mailto:Sergey.Bylokhov at oracle.com>>
>
>         Hi, Konstantin.
>         A few comments about the fix.
>         <code><em>Foo</em>Listener</code> in a few places was changed
>         to the {@code <em>Foo</em>Listener} but it is not equivalent.
>         Also I suggest to remove one of "and" from the "and and" in
>         the line 2887, because you change this line anyway.
>
>
>         On 08.08.2013 16:54, Konstantin Perikov wrote:
>>         Okay. I keep comment about synchronization for the "type"
>>         field and return comment style for transient field.
>>
>>         Is Google Drive better place for it? (that's why I ask
>>         yesterday about "good" place for webrev)
>>         https://drive.google.com/folderview?id=0B4QwwAaNe6wZUWJvTE9HRG40dnM&usp=sharing
>         No, it is not better, atleast i don't understand how to view
>         index.html in this case or download all files at once.
>
>>
>>
>>
>>         2013/8/8 Artem Ananiev <artem.ananiev at oracle.com
>>         <mailto:artem.ananiev at oracle.com>>
>>
>>             Hi, Konstantin,
>>
>>             I looked through the changes, which mostly replacements
>>             <code></code> with {@code}. This part of the fix looks fine.
>>
>>             Changing /* to /** for the "isInShow" field doesn't make
>>             sense, as this field is transient and is not serialized
>>             anyway. However, it doesn't hurt as well.
>>
>>             Please, keep comment about synchronization for the "type"
>>             field. It's a hint for developers, that this field should
>>             only be accessed or modified under the object lock. I
>>             agree, JavaDoc is not the best place for such hints, but
>>             I don't see any better solutions.
>>
>>             Non-technical comments:
>>
>>             1. Please, provide a direct link to webrev, so people can
>>             click and see the changes. Downloading archives is not as
>>             convenient (and in this particular case just impossible,
>>             as Yandex.Disk is in Russian, people just won't be able
>>             to read the "Download" button).
>>
>>             2. Please, wait for at least one more person to have
>>             reviewed this fix.
>>
>>             Thanks,
>>
>>             Artem
>>
>>             On 8/8/2013 2:20 PM, Konstantin Perikov wrote:
>>
>>                 Hi, AWT team,
>>
>>                 Could you please review the fix for the following bug:
>>
>>                 *7027045:  : (doc) java/awt/Window.java has several
>>                 typos in javadoc*
>>
>>                 Fix for version OpenJDK8. Also, I fix some
>>                 <code></code> stuff and change it for {@code }
>>
>>                 The webrev is available here:
>>
>>                 http://yadi.sk/d/KHB0hBlA7gkke
>>
>>
>>                 P.S. Since, I'm newcomer, I don't have rights to
>>                 push, so I need a sponsor. Who could help me?
>>
>>                 Thanks,
>>
>>                 Konstantin
>>
>>
>
>
>         -- 
>         Best regards, Sergey.
>
>
>


-- 
Best regards, Sergey.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20130812/1b62b44f/attachment-0001.html 


More information about the awt-dev mailing list