Looks fine to me.
I have checked the patch, have build the change and run some tests on our supported platforms.

On 30/09/2018 20:29, Tagir Valeev wrote:
Hello!

Please review JDK-8211300 [1] this change: [2]. Also it needs a
sponsor as I have only JDK author status in OpenJDK Census [3].

The discussion in core-libs-dev [4] shows that it's desired to get rid
of old style array declarations like `int array[]` and massively
convert them to `int[] array`. I volunteered to work on this. As Alan
Bateman noted [5], java.desktop module is pushed to separate repo, so
it would be better to process it separately, thus I'd like to start
with it.

The changeset was created in the following steps:
* Import JDK sources to IntelliJ IDEA
* Mark java.desktop/aix/classes, java.desktop/macosx/classes,
java.desktop/unix/classes, java.desktop/solaris/classes,
java.desktop/windows/classes and java.desktop/share/classes as source
roots
* Disable automatic formatting on the whole project
* Run the inspection "C-style array declaration"
* Apply the quick-fix massively
* Perform regex replace over changed files only:
Search: Copyright \(c\) (\d+), (\d+), Oracle and/or its affiliates.
All rights reserved.
Replace: Copyright \(c\) $1, 2018, Oracle and/or its affiliates. All
rights reserved.
* Perform regex replace over changed files only:
Search: Copyright \(c\) (\d+), Oracle and/or its affiliates. All
rights reserved.
Replace: Copyright \(c\) $1, 2018, Oracle and/or its affiliates. All
rights reserved.
* It appeared that compound declarations like `byte r[], g[], b[];`
are converted to `byte[] r;byte[] g; byte[] b;` which does not look
nice while compilable. I found three such cases via simple regexp
search in BMPImageReader#658, BMPImageWriter#325 and
AreaAveragingScaleFilter#66 and fixed them manually.

In total 339 files were changed with 1335 lines of array declaration
updates and 310 lines of copyright updates. I reviewed the generated
patch by eyes, but only partially, because it's too big. Also I
performed some kind of simple sanity check using regexps:

$ grep '^+[^+]' jdk.patch | grep -v 'Oracle and/or its affiliates. All
rights reserved'|grep -v '\[\]'|wc
       0       0       0
(check that every updated line contains either copyright message or [])

With best regards,
Tagir Valeev.

[1] https://bugs.openjdk.java.net/browse/JDK-8211300
[2] http://cr.openjdk.java.net/~tvaleev/webrev/8211300/r1/
[3] http://openjdk.java.net/census#tvaleev
[4] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-September/055390.html
[5] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-September/055759.html



--
Best regards, Sergey.

Reply via email to