[Libreoffice] [PATCH] Replace (Byte)String with O(U)String
sbergman at redhat.com
Tue Jan 17 14:13:40 PST 2012
On 01/17/2012 09:45 PM, Chr. Rossmanith wrote:
> this is a modified version of the previous patch which takes into
> account improvements suggested by Caolan and Stephan. Please don't push
> yet because the caller of RecodeString() has to be touched as well. Does
> that modification have to be contained in the same patch or do I just
> have to make sure that the two commits are pushed together? A second
> review is very welcome.
It is best to do it as a single patch (doing it in two steps would
produce problems for git bisect, for example).
The patch looks good now (only nit to pick is to consider moving from an
in--out parameter to an in-only parameter and rtl::OUString return type;
out parameters, esp. if they "hide" behind a reference type, can be
confusing -- think about somebody trying to grok where a given variable
aStr can change its value, and overlooking something innocuous-looking like
aStr = mpFontEntry->mpConversion->RecodeString(aStr);
would be so much clearer).
More information about the LibreOffice