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

Martijn Verburg martijnverburg at gmail.com
Sun Jul 1 11:05:33 UTC 2012


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!



More information about the core-libs-dev mailing list