> >> + /* No reason to choose either source or destination,
> >> + other than to exercise all code paths */
> >> + if (expand)
> >> + src_stream = svn_subst_stream_translated (src_stream,
> >> + eol_str, repair,
> >> + keywords, expand, pool);
> >> + else
> >> + dst_stream = svn_subst_stream_translated (dst_stream,
> >> + eol_str, repair,
> >> + keywords, expand, pool);
> >
> > It's not this function's job to exercise your new code paths! By all
> > means try this as a form of regression test but, for committing to the
> > repository, if you can make this function simpler by cutting out the
> > "if" and one of the paths, please do, although it seems to me that it
> > would be even simpler, and thus better, if you just left it how it was.
> > Presumably you will soon add some code that really needs the translated
> > stream, and that will exercise it.
>
> What did you think of this? Not much, it appears! (If you've replied, I
> haven't received it.)
Only after lundblad sent his review of the actual commit, I understand
what you're referring to here! :-(
Actually: I removed that change (which, lundblad, is why it's not in
the log!). I'll revert that part. Though I'd actually prefer to
deprecate svn_subst_translate_streamX() instead, since the same can be
accomplished through this stream... (but I didn't - and still don't -
want to get into that discussion; it's just not worth it.)
bye,
Erik.
Received on Wed Nov 30 16:35:31 2005