Review (NS-79): FR 1511, added localization for numbers in webpages. Almost...

Author	Henrik Kirk
Moderator	Henrik Kirk
State	Review

Objectives

952: FR 1511, added localization for numbers in webpages. Almost sure Im done

General comments:

Description

Classification

Status

|| I think the following places are missing: BitpreserveFileState.printMissingFileState() BitpreserveFileState.printChecksumErrorState() Definitions-domain-statistics.jsp (the count local variable) Translations.properties needs t obe checked for places where numbers are part of the formatted string, for instance "current.list.contains.0.urls" in the viewerproxy || Minor || OK ||

Comments on file 'trunk/webpages/HarvestDefinition/Definitions-edit-snapshot-harvest.jsp', revision 952

Lines	Description	Classification	Status
99-100, 150	As above, consider utility method.	Cosmetic	OK

Comments on file 'trunk/webpages/HarvestDefinition/Definitions-snapshot-harvests.jsp', revision 952

Lines

Description

Classification

Status

Comments on file 'trunk/webpages/HarvestDefinition/Definitions-edit-domain-config.jsp', revision 952

Lines	Description	Classification	Status
69	What happened here? It looks like to methods of listing imports are mixed up, and one breaks the "max 80 chars" rule	Cosmetic	OK
112, 146	The changes to allow formatting numbers seems a little scattered all over the place. One varaible is initialised here, one near the first use. If loc actually necessary, or could it be inlined into a single intialisation looking like NumberFormat nf = NumberFormat.getInstace(HTMLUtils.getLocaleObject(pageConext));?	Cosmetic	OK
112, 146, 150-151	Suggestion: Should formatting numbers with localization be made into a static utility method in HTMLUtils?	Cosmetic	OK
151	Wait, this doesn't make sense at all, does it? This is part of building a URL, why would you want to localize that?	Minor	OK

Comments on file 'trunk/webpages/HarvestDefinition/Definitions-add-event-seeds.jsp', revision 952

Lines	Description	Classification	Status
130	As in the last JSP file, should this be an utility method?	Cosmetic	OK