LGTM with style nits fixed.

http://codereview.chromium.org/5671002/diff/1/src/extensions/experimental/i18n-extension.cc
File src/extensions/experimental/i18n-extension.cc (right):

http://codereview.chromium.org/5671002/diff/1/src/extensions/experimental/i18n-extension.cc#newcode42
src/extensions/experimental/i18n-extension.cc:42: "    native function
NativeJSLocale();"
Please use two-space indent.

http://codereview.chromium.org/5671002/diff/1/src/extensions/experimental/i18n-extension.cc#newcode116
src/extensions/experimental/i18n-extension.cc:116: locale_name =
*v8::String::Utf8Value(args[0]->ToString());
two-space indent.

http://codereview.chromium.org/5671002/diff/1/src/extensions/experimental/i18n-extension.cc#newcode148
src/extensions/experimental/i18n-extension.cc:148: all_locales->Set(i,
v8::String::New(icu_locales[i].getName()));
two-space indent and braces around the body of the for-loop.

http://codereview.chromium.org/5671002/diff/1/src/extensions/experimental/i18n-extension.cc#newcode156
src/extensions/experimental/i18n-extension.cc:156:
std::replace(result.begin(), result.end(), '_', '-');
This is fine for an experimental extension. If this makes it as a
language feature in all browsers and we want to make this a part of the
V8 build we should attempt to get rid of the STL part. However, this
also needs ICU which is probably a bigger issue for eventual
integration. :)

http://codereview.chromium.org/5671002/diff/1/src/extensions/experimental/i18n-extension.cc#newcode172
src/extensions/experimental/i18n-extension.cc:172: return
v8::Undefined();
two space indent.

http://codereview.chromium.org/5671002/diff/1/src/extensions/experimental/i18n-extension.cc#newcode181
src/extensions/experimental/i18n-extension.cc:181: return
v8::Undefined();
two space indent.

http://codereview.chromium.org/5671002/diff/1/src/extensions/experimental/i18n-extension.cc#newcode190
src/extensions/experimental/i18n-extension.cc:190: return
v8::Undefined();
two space indent.

http://codereview.chromium.org/5671002/diff/1/src/extensions/experimental/i18n-extension.cc#newcode200
src/extensions/experimental/i18n-extension.cc:200: return
v8::Undefined();
two space indent.

http://codereview.chromium.org/5671002/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to