On Fri, Jan 22, 2010 at 02:43:27PM -0500, C. Michael Pilato wrote:
> Stefan Sperling wrote:
> > On Fri, Jan 22, 2010 at 01:04:13PM -0500, C. Michael Pilato wrote:
> >> If this is all it takes to fix this issue, I'm going to cry while deleting
> >> each and every file from the 'issue-3390-dev' branch one commit at a time.
> >> Seriously? This is the whole fix?
> >
> > Well, try it and find out :)
> > More review won't hurt.
>
> I can't seem to find any problems with this. But man, I *know* I tried this
> fix myself when I first started looking at issue #3390. Oh well, ignorance
> (or forgetfulness ... or plain ol' stupidity ...) is bliss, I guess.
>
> Oh, I know why I considered this approach sub-par now. Currently, 'svn
> switch' doesn't update any external working copies unless the switch itself
> causes the externals definitions that "own" those working copies to have a
> syntactic change (the server sends a propdiff down the wire). I was trying
> to preserve that behavior, though expanding the definition of "change" to
> include changes in the semantic interpretation of a relative-url externals
> definition. Daniel's patch (like my original pass at this issue) throws all
> that to the wind and forces Subversion to attempt to update every externals
> definition in the switched tree whether it or its interpretation changed at all.
>
> FWIW, I find Daniel's approach the sane one (because I think of update as
> merely a special-case of switch, and so expect them to behave similarly with
> respect to handling externals).
>
> I do think that there's a tiny typo in one of your test comments, Daniel:
>
> > + # Okay. We now want to switch A to A_copy, which *should* cause
> > + # A/D/ext to point to the URL for A_copy/D/ext.
>
> Shouldn't that be "...to the URL for A_copy/D." ?
Really sorry here. That's your regression test from trunk. I was just
copy-pasting it. Should have said that in the log message.
It means that all I've done for this patch is to replace NULL with
traversal_info.
> But aside from that little issue, +1 to commit. (And I'm happy to do so
> myself once Daniel affirms my recommended tweak above.)
Daniel
Received on 2010-01-22 21:31:23 CET