[Svnmerge] [SPAM] Re: [PATCH] 1/5 - #2817: clearly differentiate urls, directories, and repo-relative paths

[Dustin J. Mitchell]

On Thu, Jul 12, 2007 at 11:52:20AM +0200, Giovanni Bajo wrote:
> >@@ -1919,25 +1928,25 @@
> >         # trailing /'s.
> >         source = rstrip(source, "/")
> >         if not is_wc(source) and not is_url(source):
> >-            # Check if it is a substring of a repo-relative URL recorded
> >+            # Check if it is a substring of a pathid recorded
> >             # within the branch properties.
> >             found = []
> >-            for repos_path in branch_props.keys():
> >-                if repos_path.find(source) > 0:
> >-                    found.append(repos_path)
> >+            for pathid in branch_props.keys():
> >+                if pathid.find(source) > 0:
> >+                    found.append(pathid)
> >             if len(found) == 1:
> >+                # (XXX assumes pathid is a repository-relative-path)
> >                 source = get_repo_root(branch_dir) + found[0]
> 
> I'm not sure why you put a XXX here. What's unclear about it?

Mostly a warning to self, since in the next patch in the series that
assumption is no longer true.  I removed the 'XXX' and left the
remainder of the comment.

> >+               source_pathid == target_to_pathid("."):
> >+            error("cannot init integration source path '%s'\nIts repository-relative path must "
> >+                  "differ from the repository-relative path of the current directory." % source_pathid)
> >+        opts["source-pathid"] = source_pathid
> 
> I'd say: let's dump this repository-relative path for helping the user.

Again, this won't be the case in subsequent patches, and at this point
in the patch series, the user already has the repos-relative path
(source_pathid).

I attached a new version of the patch to bug #2817, with the changes you
suggested.  Unless there are further objections, I'll commit it
tomorrow.

Dustin

-- 
        Dustin J. Mitchell
        Storage Software Engineer, Zmanda, Inc.
        http://www.zmanda.com/