>>> Julian Foad wrote:
>>>> [...] I will be
>>>> interested in reviewing the (single) implementation.
Stefan wrote:
> [...] Here the final version.
> If that doesn't work either then I'm done for today.
Stefan: First review comment: You can much more efficiently convert a
stringbuf to a string (when you own it, like here) using
svn_stringbuf__morph_into_string(). Second review question: did you
consider whether the second implementation was in fact already better?
For example, is using _appendbytes better or worse than using
ensure(size x 2) followed by read()?
Ivan wrote:
> Merging svn_stringbuf_from_stream() and svn_stringbuf_from_stream()
> implementations is completely ortogonal from my point of view.
>
> But I really don't understand what was your point in r1707196 review
> if you are not interested in review any changes to this function at
> all.
Ivan: I reviewed your commit because I saw it. During review I *then*
thought "surely we must already have some code for doing this" and so
I went looking for similar code and found the existing similar
function. And combining them is not orthogonal to tweaking one of them
on its own. (Combining them would be orthogonal to tweaking both of
the implementations together, if the implementations were initially
similar enough so that would make sense.) Tweaking one of them on its
own doesn't give me any clue whether you have considered whether the
other implementation might already be better than what you're doing.
- Julian
Received on 2015-10-07 15:22:00 CEST