On Apr 13 2013, at 09:34 , Ulf Zibis wrote: > Am 12.04.2013 23:36, schrieb Mike Duigou: >> On Apr 11 2013, at 15:15 , Ulf Zibis wrote: >> >>> There is still a yoda style in ConcurrentMap line 72, HashMap line 361 >> Fixed > > I still see no change in webrev 5 in HashMap line 361
Now corrected. > >> >>> To be in line with old habits, please remove space after casts. See also: >>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6939278 >> I am not going to do this. > > Well in Hashtable, existing code doesn't have the space after cast. If you > insert space in new code, you introduce an inconsistency in style. It seems that, per your bug, everyone has given up hope that spacing after casts will be done consistently. I've corrected a few cases in new code but am not terribly concerned about this. > Even your new code is inconsistent, compare: That's because I'm not the only author. I get to fix it though, the glamourous life of a JDK janitor. :-) > HashTable line 917, 938, 984 etc. > HashMap line 588 etc. > Collections line 1402 etc. Should be more consistent now. I'm not willing to attempt correct this non-problem more broadly. > > I also notice, you introduce a new style ? in for( ; ... ; ... ;) expressions > with space before ';'. Fixed in HashMap. I don't see this anywhere else. > >>> Collections: >>> lines 1442... + 3900... , indentation for follow up line should be 8 > > There are still lines 976, 1062 Of which file? According to http://cr.openjdk.java.net/~mduigou/JDK-8010122/5/webrev/src/share/classes/java/util/Collections.java.html neither of those lines seems applicable. > >>> Map: >>> lines 599..600 bad indentation >> Corrected. > > Hm, I see still 3 instead 4 spaces and also in lines 545..546 Both corrected. > >> >>> Use consistent formatted code examples in javadoc. I like the style from >>> lines 567 to 570, but e.g. from line 622 to 626: >>> - 2 spaces before <pre> >>> - indentation should be 4 >>> - line break before }</pre> >> I had left these along in the previous round. Should be fixed now. > > Great, but see also code samples in ConcurrentMap. I'm not touching those. We are dependent upon syncs from JSR-166 repo for this class and don't want to introduce any extraneous changes. Try to convince Doug if you want. Mike
