[prev in list] [next in list] [prev in thread] [next in thread] 

List:       subversion-dev
Subject:    Re: [PATCH] Add a keyword+eol translating stream
From:       Erik Huelsmann <ehuels () gmail ! com>
Date:       2005-11-30 14:52:01
Message-ID: aea328ab0511300652k68dd039bv32ee88896782e991 () mail ! gmail ! com
[Download RAW message or body]

> > > +  /* 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 understandwhat you're \
                referring to here! :-(
Actually: I removed that change (which, lundblad, is why it's not inthe log!). I'll \
revert that part. Though I'd actually prefer todeprecate \
svn_subst_translate_streamX() instead, since the same can beaccomplished 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.


[prev in list] [next in list] [prev in thread] [next in thread]