[Svnmerge] request for review: #2862 - cleanup pathid abstraction
Giovanni Bajo
rasky at develer.com
Sat Aug 4 04:25:13 PDT 2007
On ven, 2007-08-03 at 16:21 -0500, Dustin J. Mitchell wrote:
> This patch cleans up the pathid abstraction in preparation for adding
> multiple underlying pathid representations.
I keep forgetting what you're aiming at... :)
Full support for merges across multiple repositories?
Your patch looks OK. There's only a couple of nits:
- This change:
> @@ -806,10 +809,14 @@
> # Try using "svn info URL". This works only on SVN clients >= 1.2
> try:
> info = get_svninfo(url)
> - return info["Repository Root"]
> except LaunchError:
> pass
>
> + try:
> + return info["Repository Root"]
> + except KeyError:
> + pass
> +
is unrelated, AFAICT. Also I don't understand it: when is it possible
that info does not contain the Repository Root?
- If you're going to totally abstract away pathid, why not making it a
little (immutable) class? It might end up being a little more readabale
(pathid.to_url(), pathid.path, ecc.).
- I don't undesrtand the *targets disambiguation arguments, but I guess
those will be clearer in the next patch :)
--
Giovanni Bajo
More information about the Svnmerge
mailing list