Boost logo

Boost :

Subject: Re: [boost] [locale] Review of the proposed Boost.Locale library
From: Artyom (artyomtnk_at_[hidden])
Date: 2011-04-18 17:07:34

> From: Matus Chochlik <chochlik_at_[hidden]> > > - Please explicitly state in your review whether the library should be > accepted. > > Yes, it should be accepted. > I'm glad to hear. > [snip] > > AFAIK the Boost Coding Guidelines require max 80 > characters per line in the source code. The sources > of Boost.Locale do not follow this guideline which > makes them slightly less readable, one needs > to scroll a lot horizontally. Otherwise I leave the quality > of the implementation to be judged by more competent > people on this list. > Small point, there are lots of code in boost that does not follow 80 characters limit. Also I understand the desire to put somewhere a limit (derived from the size of a punch card back to 60th) But sometimes it just make it more then less readable. > [snip] > > - What is your evaluation of the documentation? > > The documentation is readable and clear to someone > familiar with l10n/i18n however things like collation, > capitalization, case folding, etc. should be better > explained for users who are not familiar with them. > The previous/next page links are missing which is > little inconvenient. Noted. > > In the Message Formatting/Message Translation section > there is a typo: > ------------------------------------------------------------------------------ > std::cout << boost::locale::tanslate("Hello World!") << std::endl; > ------------------------------------------------------------------------------ > Noted. > In the examples and code snippets in the docs I think > that "using namespace std;" should be removed > and instead of > "using namespace boost::locale;" > > there should be something like > "namespace bl = boost::locale;" > > which would make much clearer which parts of the sample code > are referring to Boost.Locale's code and which to std library; Good point. > > Also (but I might be mistaken here since I'm not a native > German speaker) the example in the "Conversions" > section names a variable 'gruben' (this variable stores > the word "grüßen") a German would probably > call this variable 'grussen' since 'ß' != 'b'. > Yes, you are right grussen is better. > [snip] > > - Did you try to use the library? With what compiler? Did you have any > problems? > > Yes (I built the library), > - on Debian Linux (64bit) with ICU 4.6 > - using cmake > - with GCC 4.6.0 non-C++0x mode: no problems > - with GCC 4.6.0 C++0x mode: auto_ptr - related warnings > - using bjam (Boost 1.46.1) > - with GCC 4.6.0 non-C++0x mode: no problems > - on Windows Vista Basic (32-bit) + cygwin + ICU 4.5 > - using bjam (Boost 1.46.1) > - with GCC 4.3.4: no problems > - on Windows Vista (32-bit) + ICU 4.6 (bin package for Win32) > - using bjam (Boost 1.46.1) > - with MSVC 9: the library built, but for some reason > bjam didn't use ICU even with -sICU_PATH specified, > and I didn't have the time to investigate why. > It may be due to fact that you used provided ICU packages (for MSVC10 and they release only) It is always better to build ICU by yourself. They provide visual studio projects. I probably need to state this more explicitly in tutorial - how to build ICU with MSVC. > [snip] > > > Best regards, > > Matus Chochlik Thank you very much for the review. Artyom

Boost list run by bdawes at, gregod at, cpdaniel at, john at