cldr-import mostly looks good, but there are a few consistent issues.


http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/DateTimeFormatInfoProcessor.java
File
tools/cldr-import/src/com/google/gwt/tools/cldr/DateTimeFormatInfoProcessor.java
(right):

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/DateTimeFormatInfoProcessor.java#newcode52
tools/cldr-import/src/com/google/gwt/tools/cldr/DateTimeFormatInfoProcessor.java:52:
new String[]{"sun", "mon", "tue", "wed", "thu", "fri", "sat"};
Why is it dropping the space before "{"?  Also, the line break seems
odd, though I could live with it.

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/DateTimeFormatInfoProcessor.java#newcode320
tools/cldr-import/src/com/google/gwt/tools/cldr/DateTimeFormatInfoProcessor.java:320:
fallbackCategory == null ? Collections.<String, String> emptyMap() :
localeData.getEntries(
Also here the line break seems weird.

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/DateTimeFormatInfoProcessor.java#newcode396
tools/cldr-import/src/com/google/gwt/tools/cldr/DateTimeFormatInfoProcessor.java:396:
+ "dayPeriodContext[@type=\"format\"]/"
Why does this get indended again?  If it is consistent I can live with
it, but it seems wrong.

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/GenerateGwtCldrData.java
File
tools/cldr-import/src/com/google/gwt/tools/cldr/GenerateGwtCldrData.java
(right):

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/GenerateGwtCldrData.java#newcode59
tools/cldr-import/src/com/google/gwt/tools/cldr/GenerateGwtCldrData.java:59:
UOption[] options =
Wow, this one is really ugly:  It splts the assignment in a weird place,
it puts the opening brace on its own line yet puts the closing brace at
the end (with no space even).

I definitely think this needs more tweaking.

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/LocaleData.java
File tools/cldr-import/src/com/google/gwt/tools/cldr/LocaleData.java
(right):

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/LocaleData.java#newcode432
tools/cldr-import/src/com/google/gwt/tools/cldr/LocaleData.java:432:
"type");
Why does it increase the indent level and then go back?

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/LocaleData.java#newcode633
tools/cldr-import/src/com/google/gwt/tools/cldr/LocaleData.java:633:
.fromString(localeName);
Yuck - I greatly prefer splitting method calls only when necessary to
make it fit, rather than just so it can fit one more work on the
previous line.

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/LocaleData.java#newcode653
tools/cldr-import/src/com/google/gwt/tools/cldr/LocaleData.java:653: *
      specified category.
Previously, the autoformatter was set to indent 4 characters for wrapped
Javadoc, which was the original text.  Now it seems like it is back to
the default of lining it up with the first line.

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/LocalizedNamesProcessor.java
File
tools/cldr-import/src/com/google/gwt/tools/cldr/LocalizedNamesProcessor.java
(right):

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/LocalizedNamesProcessor.java#newcode100
tools/cldr-import/src/com/google/gwt/tools/cldr/LocalizedNamesProcessor.java:100:
.getVariantNotNull());
Another awkward split.

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/LocalizedNamesProcessor.java#newcode339
tools/cldr-import/src/com/google/gwt/tools/cldr/LocalizedNamesProcessor.java:339:
.getRegions(locale.getLanguageNotNull() + "_" +
locale.getScriptNotNull());
Yikes.  Bad assignment split, and splitting at . rather than in the
argument list looks really bad.

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/Processor.java
File tools/cldr-import/src/com/google/gwt/tools/cldr/Processor.java
(right):

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/src/com/google/gwt/tools/cldr/Processor.java#newcode142
tools/cldr-import/src/com/google/gwt/tools/cldr/Processor.java:142: new
BufferedWriter(new OutputStreamWriter(new FileOutputStream(f),
"UTF-8")), false);
Another really bad split.

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java
File
tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java
(right):

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java#newcode43
tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java:43:
new LocaleData(localeFactory, Arrays.asList("root", "en", "en_US", "ar",
"ar_IQ"));
again

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java#newcode61
tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java:61:
new LocaleData(localeFactory, Arrays.asList("root", "en", "en_US", "ar",
"ar_IQ"));
again

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java#newcode85
tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java:85:
new LocaleData(localeFactory, Arrays.asList("root", "en", "en_US", "ar",
"ar_IQ"));
again

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java#newcode117
tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java:117:
"ar_IQ"));
again

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java#newcode140
tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java:140:
GwtLocale localeEn = localeFactory.fromString("en");
again

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java#newcode160
tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java:160:
new LocaleData(localeFactory, Arrays.asList("root", "en", "en_US", "ar",
"ar_IQ"));
again

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java#newcode180
tools/cldr-import/test/com/google/gwt/tools/cldr/LocaleDataTest.java:180:
new LocaleData(localeFactory, Arrays.asList("root", "en", "en_US", "ar",
"ar_IQ"));
again

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/datetimefmtcreator/src/com/google/gwt/tools/datetimefmtcreator/DateTimeFormatCreator.java
File
tools/datetimefmtcreator/src/com/google/gwt/tools/datetimefmtcreator/DateTimeFormatCreator.java
(right):

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/datetimefmtcreator/src/com/google/gwt/tools/datetimefmtcreator/DateTimeFormatCreator.java#newcode60
tools/datetimefmtcreator/src/com/google/gwt/tools/datetimefmtcreator/DateTimeFormatCreator.java:60:
.getBestPattern(entry.getValue())});
bad split, lost space

http://gwt-code-reviews.appspot.com/1402803/diff/1/tools/datetimefmtcreator/src/com/google/gwt/tools/datetimefmtcreator/DateTimeFormatCreator.java#newcode423
tools/datetimefmtcreator/src/com/google/gwt/tools/datetimefmtcreator/DateTimeFormatCreator.java:423:
*         value
changing javadoc indent

http://gwt-code-reviews.appspot.com/1402803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to