RFR (JDK10/JAXP) 8181155: Fix lint warnings in JAXP repo: fallthrough and static
Joe Wang
huizhe.wang at oracle.com
Mon Oct 30 19:59:04 UTC 2017
On 10/30/17, 12:22 PM, joe darcy wrote:
> Hi Joe,
>
> As an alternative, consider documenting the semantics/correctness of
> the fall-through at the location of the fall-through rather than at
> the top of the switch or method.
Yes, in many of these cases, there already have been comments such as
"fall through" at the location of the fall-through. But since some of
the methods were very long, I thought it would make it easies for others
who might read these method to understand by searching the keywords
"case ...". The ones I added then served as an additional pointer to
where fallthrough happens.
Thanks,
Joe
>
> Thanks,
>
> -Joe
>
>
> On 10/30/2017 12:17 PM, Joe Wang wrote:
>>
>>
>> On 10/30/17, 11:14 AM, Roger Riggs wrote:
>>> Hi Joe,
>>>
>>> +1
>>>
>>> Is there a useful comment on the @SuppressWarnings like in other files:
>>>
>>> XSDHandler.java: 3028
>>> DTMDocumentImpl: 1700
>>> DOM2DTM.java: 1654
>>> FilterExprWalker.java: 65
>>
>> For the methods with long or very long switch statements, I added a
>> note following the SuppressWarnings annotation to indicate where
>> fallthrough would happen and in which case warnings were suppressed.
>> But for the pretty short ones like the above, I thought it's quite
>> obvious where fallthrough might happen, I didn't therefore add any
>> comment.
>>
>>>
>>> A few of the added breaks would have been hiding bugs.
>>> It might be worth mentioning them in the issue.
>>
>> I added a note to the issue.
>>
>> Thanks,
>> Joe
>>
>>>
>>> Roger
>>>
>>>
>>>
>>> On 10/30/2017 1:09 PM, Lance Andersen wrote:
>>>> Hi Joe
>>>>
>>>> The changes look OK
>>>>
>>>> Best
>>>> Lance
>>>>> On Oct 30, 2017, at 1:03 PM, Joe Wang <huizhe.wang at oracle.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Please review a cleanup of fallthrough and static warnings. For
>>>>> fallthrough, the majority of the changes are suppressing the
>>>>> warnings, while for static, replacing the instances with the class.
>>>>>
>>>>> All jaxp tests and JCK passed.
>>>>>
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8181155
>>>>> webrevs:
>>>>> http://cr.openjdk.java.net/~joehw/jdk10/8181155/webrev/index.html
>>>>>
>>>>> Thanks,
>>>>> Joe
>>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance
>>>> Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>>> Oracle Java Engineering
>>>> 1 Network Drive
>>>> Burlington, MA 01803
>>>> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
>>>>
>>>>
>>>>
>>>
>
More information about the core-libs-dev
mailing list