Hi Kumar,
I reviewed your latest webrev.02. Looks good to me.
Thanks
Mandy
On 11/12/10 12:17, Kumar Srinivasan wrote:
Thanks for all the reviews!.
Here are the fixes in this version:
http://cr.openjdk.java.net/~ksrini/6452854/webrev.01
1. Fixed the representation of numbers and scaling using BigDecimal.
2. Argument processing checks for -XshowSettings and -XshowSetting:opt
(added a test to catch the former, added some comments in java.c)
3. Replaced String* with PrintStream.print*, it makes the logic easier
to read, so I decided to go with this.
4. Removed extraneous path.separators.
5. line.separator will be printed as ASCII symbols CR and LF
6. If the helper cannot determine the value that line will not be
displayed.
7. In the case of the Max memory not gotten from the launcher
(Estimated) is printed.
8. Removed the option hints -X* and so on in the labels.
9. Renamed Ergonomic Class -> Ergonomic Machine Class.
Thanks
Kumar
On 11/11/2010 16:42, Kumar Srinivasan wrote:
line 176, 188, 190-191, 195, and other lines in printPrintLocales
and printLocale methods:
- the assignment to the buf and out variable to itself (returned
from StringBuffer.append() method) is not necessary.
Yes fixed, I missed these.
The "intention" of returning the StringBuffer itself for those
append() methods is that
you can then write the code like
private static void printLocale(PrintStream ostream) {
ostream.println(new StringBuilder("\n" + LOCALE_SETTINGS + "\n")
.append(INDENT)
.append("default locale: ")
.append(Locale.getDefault().getDisplayLanguage())
.append(prettyPrintLocales())
.toString());
}
One more nit is that you might want to do something special for
line.separator =
to make it "readable"
-Sherman
