Bug 7176907 - Patches for javac warnings cleanup (text and util) from Adopt OpenJDK

[Stuart Marks]

Pushed:

http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b2fc66012451

Thanks for your contributions to OpenJDK!

s'marks

On 7/1/12 4:05 AM, Martijn Verburg wrote:
> Hi Stuart,
>
>> <snip>
>>
>> The java.util patches look good and are almost ready to go in. The only
>> issue is how to format the Contributed-by line in the changeset comment.
>> What I have so far for the comment is:
>>
>> 7176907: additional warnings cleanup in java.util, java.util.regexp,
>> java.util.zip
>> Reviewed-by: forax, khazra, smarks
>> Contributed-by: ???
>>
>> The contributed-by line takes one or more email addresses or email/name
>> pairs. For an earlier contribution from LJC, see here [1]. This isn't a
>> terribly big deal but I want to make sure that credit goes where it's due.
>> Can you tell me what I should put for the contributed-by line?
>
> Main Sarkar - sadhak001ATgmail.DOTcom
>
>> The warnings in java.text have already been fixed by Deepak Bhole's
>> changeset [2]. Not a problem, this took two minutes to figure out.
>
> Cool, we've adjusted our workflow to take this into account.
>
>> There were a couple questions from earlier in the thread.
>>
>> On the discussion of when the compiler issues switch fallthrough warnings,
>> from what I can tell, the compiler issues a warning whenever there is actual
>> code in a case that doesn't end with break, continue, return, or throw. This
>> seems independent of whether what follows is another case or the 'default'
>> case. If there are several case clauses together with no intervening code,
>> this isn't considered a fallthrough and thus there is no warning. This make
>> sense, as sometimes you want several different cases to be handled by the
>> same code. For example,
>>
>>      switch (ch) {
>>          case 'a':
>>              // no warning here
>>          case 'b':
>>              someActualWork();
>>              break;
>>          // ...
>>      }
>
> Understood, made sense to us as well.
>
>> Regarding whether there is a style checker for indentation and spacing, I'm
>> not aware of a good one that I can recommend. We generally adhere to the
>> (very old) Java Coding Conventions [3]. I think most people just deal with
>> style issues manually by hand and by eye; I know I do. We do run jcheck [4]
>> on every incoming changeset, but the only things it checks in files'
>> contents are for trailing whitespace, and CR (as opposed to LF) and TAB
>> characters.
>
> OK, we've added jcheck to the list of things to run going forwards.
> Perhaps in the
> future one of our folks can put together a checkstyle ruleset for
> core-lib (controversial!)...
> :-)
>
> Thanks for getting this patch merged!