Thanks for updating this. This looks good to me. I guess Stuart will be
sponsoring the patch.
There are a couple of other switch statements in
src/share/classes/java/util/regex/Pattern.java.
which are not throwing fallthrough warnings (in Netbeans at least),
although there is fallthrough happening from one case to another. From
what I notice, only if there is a break statement
missing before the "default" case, does Netbeans throw some warning. Is
the fallthrough warning technically
supposed to be thrown only when the logic falls through to the default
case then?
- Kurchi
On 6/25/2012 5:54 AM, Martijn Verburg wrote:
Hi all,
Apologies for the delay. So it was simply a case of human error in
missing that last fallthrough (we wanted to double check that our
warnings script wasn't failing, hence the delay in getting back to
you). I've respun the patch with the extra SuppressWarning.
Hopefully the patch is complete now.
Cheers,
Martijn
On 20 June 2012 17:18, Martijn Verburg<[email protected]> wrote:
We'll look into it, hopefully have an answer for you shortly - M
On 20 June 2012 17:07, Kurchi Subhra Hazra
<[email protected]> wrote:
Hi,
I was just going through the patches, there are some more fallthrough
cases in
src/share/classes/java/util/regex/Pattern.java.(for example in line 2247).
Are these not generating warnings?
- Kurchi
On 6/20/12 7:30 AM, Martijn Verburg wrote:
Hi all,
Apologies, I didn't check that attachments were stripped. The patches
can be found at:
https://raw.github.com/AdoptOpenJDK/PatchReview/master/submitted/core_java_text.patch
https://raw.github.com/AdoptOpenJDK/PatchReview/master/submitted/core_java_util.patch
Cheers,
Martijn
Hi Martijn,
the two patches looks good.
A minor nit, why is there a space between the '(' and the readUByte() in
readUShort.
Thanks for the quick review! No reason on the whitespace, I've fixed that
now.
Quick question. Is there a checkstyle or jcheck that we should be
applying to any corelib patches going forwards?
Cheers,
Martijn
--
-Kurchi
