Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(403)

Issue 6498107: code review 6498107: exp/locale/collate: changed API to allow access to diff... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 6 months ago by mpvl
Modified:
11 years, 6 months ago
Reviewers:
rog
CC:
r, golang-dev
Visibility:
Public.

Description

exp/locale/collate: changed API to allow access to different locales through New(), instead of variables. Several reasons: - Encourage users of the API to minimize the number of creations and reuse Collate objects. - Don't rule out the possibility of using initialization code for collators. For some locales it will be possible to have very compact representations that can be quickly expanded into a proper table on demand. Other changes: - Change name of root* vars to main*, as the tables are shared between locales. - Added Locales() method to get a list of supported locales.

Patch Set 1 #

Patch Set 2 : diff -r 0a0666f2fe86 https://go.googlecode.com/hg #

Patch Set 3 : diff -r 5b1777d3a58e https://go.googlecode.com/hg #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+398 lines, -46 lines) Patch
M src/pkg/exp/locale/collate/build/builder.go View 1 1 chunk +21 lines, -4 lines 0 comments Download
M src/pkg/exp/locale/collate/build/table.go View 1 1 chunk +15 lines, -0 lines 0 comments Download
M src/pkg/exp/locale/collate/collate.go View 1 1 chunk +21 lines, -0 lines 2 comments Download
M src/pkg/exp/locale/collate/maketables.go View 1 3 chunks +1 line, -17 lines 0 comments Download
M src/pkg/exp/locale/collate/table.go View 1 2 chunks +14 lines, -0 lines 0 comments Download
M src/pkg/exp/locale/collate/tables.go View 1 6 chunks +326 lines, -25 lines 0 comments Download

Messages

Total messages: 5
mpvl
Hello r@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg
11 years, 6 months ago (2012-09-08 04:27:36 UTC) #1
r
LGTM
11 years, 6 months ago (2012-09-13 17:13:48 UTC) #2
mpvl
*** Submitted as http://code.google.com/p/go/source/detail?r=600708342ddd *** exp/locale/collate: changed API to allow access to different locales through ...
11 years, 6 months ago (2012-09-14 10:10:07 UTC) #3
rog
http://codereview.appspot.com/6498107/diff/6001/src/pkg/exp/locale/collate/collate.go File src/pkg/exp/locale/collate/collate.go (right): http://codereview.appspot.com/6498107/diff/6001/src/pkg/exp/locale/collate/collate.go#newcode90 src/pkg/exp/locale/collate/collate.go:90: return availableLocales just noticed this, sorry. i think this ...
11 years, 6 months ago (2012-09-14 11:23:15 UTC) #4
mpvl
11 years, 6 months ago (2012-09-24 04:13:17 UTC) #5
http://codereview.appspot.com/6498107/diff/6001/src/pkg/exp/locale/collate/co...
File src/pkg/exp/locale/collate/collate.go (right):

http://codereview.appspot.com/6498107/diff/6001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate.go:90: return availableLocales
Will add comment in a next CL.
Thanks

On 2012/09/14 11:23:15, rog wrote:
> just noticed this, sorry. i think this function should either document that
the
> returned slice should not be modified, or it should return a copy. i don't
> particularly mind which.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b