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

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

Hi Stuart,

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