FWIW, my concerns here are:

* svn_client_relocate{,2} have the same signature.  This might be
  confusing sometimes. (but probably should be left alone)

* svn_client_relocate2() takes an IGNORE_EXTERNALS parameter.  Should
  we pass TRUE always to that parameter, or should we pass the
  identically-named parameter of the calling function? (the calling
  function *happens* to have an appropriately-named parameter, but
  I haven't checked its semantics)

Daniel


Julian Foad wrote on Wed, Dec 08, 2010 at 15:09:32 +0000:
> On Wed, 2010-12-08, Philip Martin wrote:
> > Julian Foad <julian.f...@wandisco.com> writes:
> > 
> > > On Wed, 2010-12-08 at 14:26 +0000, Julian Foad wrote:
> > >> Prabhu Gnana Sundar wrote:
> > >> > I have attached a patch with a minor change which fixes a compiler
> > >> > warning.
> > >> 
> > >> Hi Prabhu.  How do you know that svn_client_relocate2() is a drop-in
> > >> replacement for svn_client_relocate() in this case?  What is difference
> > >> between svn_client_relocate() and svn_client_relocate2()?  Does it
> > >> matter?  Can you think of any way of testing or verifying it?
> > >
> > > Did you run the test suite?  Does the test suite exercise this code
> > > path?
> > 
> > That's the http redirect code, tested by redirect_tests.py.  The patch
> > is correct.
> 
> Great.  Thanks, Philip.  I over-reacted and made a mistake.  I
> immediately looked at the doc string of svn_client_relocate() and saw
> the words "dir is required to be a working copy root", and mis-read it
> as specifying a restriction of relocate2() compared to relocate().  As I
> did not know whether Prabhu had verified or tested anything, that
> misunderstanding made me suspect that the patch was broken.
> 
> Prabhu: I'm sorry I responded with a barrage of questions.  It appears
> your patch is fine.
> 
> In the future, if you could say just a few words about what
> investigation and/or testing you have done, each time you submit a
> patch, that would help me to know what level of confidence I should have
> when I start looking at it.  Thanks in advance.
> 
> And in the future I'll try not to be so hasty in my responses.
> 
> - Julian
> 
> 

Reply via email to