Merge lp:~luoyonggang/bzr-svn/parse_revnum into lp:bzr-svn/1.1

[Yonggang Luo]

Add revnum parameters on some functions.

[Jelmer Vernooij (jelmer)]

Hi,

there is an extra space between revnum and ")" in one of the parse methods.

Can you please also add a repository argument, for consistency?

review: Needs Fixing

[Yonggang Luo (luoyonggang)]

>
> Review: Needs Fixing
> Hi,
>
> there is an extra space between revnum and ")" in one of the parse methods.
>
> Can you please also add a repository argument, for consistency?
>
Excuse me, I think such kinds of format problem fixed by the reviewer is
more time saving.
anyway, this time I'll fix it up.

> --
> https://code.launchpad.net/~luoyonggang/bzr-svn/parse_revnum/+merge/70105
> You are the owner of lp:~luoyonggang/bzr-svn/parse_revnum.
>

--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

[Yonggang Luo (luoyonggang)]

which function?

[Yonggang Luo (luoyonggang)]

Ok

[Jelmer Vernooij (jelmer)]

On Tue, 2011-08-02 at 09:11 +0000, Yonggang Luo wrote:
> which function?
All the ones where you've added revnum arguments. Usually layouts don't
have a repository object of their own, so if you want to pass the revnum
to the layout you have to provide the right context.

Cheers,

jelmer

[Jelmer Vernooij (jelmer)]

On Tue, 2011-08-02 at 09:06 +0000, Yonggang Luo wrote:
> >
> > Review: Needs Fixing
> > Hi,
> >
> > there is an extra space between revnum and ")" in one of the parse methods.
> >
> > Can you please also add a repository argument, for consistency?
> >
> Excuse me, I think such kinds of format problem fixed by the reviewer is
> more time saving.
> anyway, this time I'll fix it up.
Except I'm not merging this just yet, and have no idea if I'll spot this
issue next time. You have to touch this branch anyway (see my other
comments), so it seems reasonable to ask you to fix it.

Cheers,

Jelmer

[Yonggang Luo (luoyonggang)]

2011/8/2 Jelmer Vernooij <email address hidden>

> On Tue, 2011-08-02 at 09:11 +0000, Yonggang Luo wrote:
> > which function?
> All the ones where you've added revnum arguments. Usually layouts don't
> have a repository object of their own, so if you want to pass the revnum
> to the layout you have to provide the right context.
>
> Right context?
What's that??

> Cheers,
>
> jelmer
>
> --
> https://code.launchpad.net/~luoyonggang/bzr-svn/parse_revnum/+merge/70105
> You are the owner of lp:~luoyonggang/bzr-svn/parse_revnum.
>

--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

[Yonggang Luo (luoyonggang)]

Well, I have already fixup the space problem, any other problem?

[Jelmer Vernooij (jelmer)]

Can you add a repository argument as well in the places where you are adding a revnum argument?

[Yonggang Luo (luoyonggang)]

Er... OK.

2011/8/4 Jelmer Vernooij <email address hidden>

> Can you add a repository argument as well in the places where you are
> adding a revnum argument?
> --
> https://code.launchpad.net/~luoyonggang/bzr-svn/parse_revnum/+merge/70126
> You are the owner of lp:~luoyonggang/bzr-svn/parse_revnum.
>

--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

[Jelmer Vernooij (jelmer)]

> Er... OK.

Layout implementations don't usually have a reference to the repository. Passing in the revnum *and* the repository means that other people who implement custom layouts will be able to use revision-specific data. It also means we'll only have to change the existing API only once (I have no idea if and how many custom bzr plugins there are out there that add custom layout implementations).

Unmerged revisions

3789. By Yonggang Luo on 2011-08-02

Remove extra space.

3788. By Yonggang Luo on 2011-08-02

Add revnum parameter for
parse and split_project_path

=== modified file 'branch.py'
--- branch.py	2011-07-21 13:42:21 +0000
+++ branch.py	2011-08-02 09:16:13 +0000
@@ -226,7 +226,7 @@
                     raise NotBranchError(self.base)
                 raise
         try:
-            (type, self.project, _, ip) = self.layout.parse(branch_path)
+            (type, self.project, _, ip) = self.layout.parse(branch_path, revnum)
         except NotSvnBranchPath:
             raise NotBranchError(branch_path)
         if type not in ('branch', 'tag') or ip != '':
 
=== modified file 'layout/__init__.py'
--- layout/__init__.py	2011-06-29 15:45:06 +0000
+++ layout/__init__.py	2011-08-02 09:16:13 +0000
@@ -80,19 +80,19 @@
         """
         raise NoCustomBranchPaths(self)
 
-    def parse(self, path):
-        """Parse a path.
+    def parse(self, path, revnum):
+        """Parse a path with revnum.
 
         :return: Tuple with type ('tag', 'branch'), project name, branch path and path
             inside the branch
         """
         raise NotImplementedError
 
-    def split_project_path(self, path, project):
-        """Parse a project inside a particular project.
+    def split_project_path(self, path, project, revnum):
+        """Parse a project inside a particular project at a particular revision.
 
         """
-        (pt, parsed_project, bp, ip) = self.parse(path)
+        (pt, parsed_project, bp, ip) = self.parse(path, revnum)
         if project is not None and parsed_project != project:
             raise NotSvnBranchPath(path, self)
         return (pt, bp, ip)
@@ -100,7 +100,7 @@
     def is_branch(self, path, project=None):
         """Check whether a specified path points at a branch."""
         try:
-            (type, proj, bp, rp) = self.parse(path)
+            (type, proj, bp, rp) = self.parse(path, None)
         except NotSvnBranchPath:
             return False
         if (type == "branch" and rp == "" and
@@ -111,7 +111,7 @@
     def is_tag(self, path, project=None):
         """Check whether a specified path points at a tag."""
         try:
-            (type, proj, bp, rp) = self.parse(path)
+            (type, proj, bp, rp) = self.parse(path, None)
         except NotSvnBranchPath:
             return False
         if (type == "tag" and rp == "" and
 
=== modified file 'layout/standard.py'
--- layout/standard.py	2011-07-26 14:18:15 +0000
+++ layout/standard.py	2011-08-02 09:16:13 +0000
@@ -83,7 +83,7 @@
         else:
             return urlutils.join(project, "branches", name).strip("/")
 
-    def parse(self, path):
+    def parse(self, path, revnum):
         """Parse a path.
 
         :return: Tuple with type ('tag', 'branch'), project name, branch path and path
@@ -190,7 +190,7 @@
             raise svn_errors.NoCustomBranchPaths(self)
         return ""
 
-    def parse(self, path):
+    def parse(self, path, revnum):
         """Parse a path.
 
         :return: Tuple with type ('tag', 'branch'), project name, branch path and path
@@ -250,7 +250,7 @@
         """
         return None
 
-    def parse(self, path):
+    def parse(self, path, revnum):
         """Parse a path.
 
         :return: Tuple with type ('tag', 'branch'), project name, branch path and path
@@ -367,7 +367,7 @@
                 return True
         return False
 
-    def parse(self, path):
+    def parse(self, path, revnum):
         """Parse a path.
 
         :return: Tuple with type ('tag', 'branch'), project name, branch path
@@ -436,7 +436,7 @@
         """
         return urlutils.join("branches", project, name).strip("/")
 
-    def parse(self, path):
+    def parse(self, path, revnum):
         """Parse a path.
 
         :return: Tuple with type ('tag', 'branch'), project name, branch path and path
 
=== modified file 'mapping2.py'
--- mapping2.py	2011-07-26 14:18:15 +0000
+++ mapping2.py	2011-08-02 09:16:13 +0000
@@ -206,7 +206,7 @@
         super(TrunkLegacyLayout, self).__init__()
         self.level = level
 
-    def parse(self, path):
+    def parse(self, path, revnum):
         parts = path.strip("/").split("/")
         if len(parts) == 0 or self.level >= len(parts):
             raise NotSvnBranchPath(path, self)
@@ -247,7 +247,7 @@
 
 class RootLegacyLayout(LegacyLayout):
 
-    def parse(self, path):
+    def parse(self, path, revnum):
         return ("branch", "", "", path)
 
     def is_branch(self, path, project=None):
 
=== modified file 'mapping3/base.py'
--- mapping3/base.py	2011-07-26 14:18:15 +0000
+++ mapping3/base.py	2011-08-02 09:16:13 +0000
@@ -54,7 +54,7 @@
         self.repository = repository
         self.scheme = scheme
 
-    def parse(self, path):
+    def parse(self, path, revnum):
         try:
             (proj, bp, rp) = self.scheme.unprefix(path)
         except InvalidSvnBranchPath, e:
 
=== modified file 'metagraph.py'
--- metagraph.py	2011-07-16 10:05:48 +0000
+++ metagraph.py	2011-08-02 09:16:13 +0000
@@ -441,7 +441,7 @@
                     continue
                 action = paths[p][0]
                 try:
-                    (_, bp, ip) = self.layout.split_project_path(p, self._project)
+                    (_, bp, ip) = self.layout.split_project_path(p, self._project, revnum)
                 except svn_errors.NotSvnBranchPath:
                     pass
                 else:
 
=== modified file 'push.py'
--- push.py	2011-07-26 09:57:59 +0000
+++ push.py	2011-08-02 09:16:13 +0000
@@ -480,7 +480,7 @@
             if base_foreign_revid is None:
                 target_project = None
             else:
-                (_, target_project, _, _) = layout.parse(base_foreign_revid[1])
+                (_, target_project, _, _) = layout.parse(base_foreign_revid[1], base_foreign_revid[2])
             bp = determine_branch_path(rev, layout, target_project)
             target_config = self._get_branch_config(bp)
             push_merged = (layout.push_merged_revisions(target_project) and
@@ -596,7 +596,7 @@
                 if base_foreign_revid is None:
                     target_project = None
                 else:
-                    (_, target_project, _, _) = layout.parse(base_foreign_revid[1])
+                    (_, target_project, _, _) = layout.parse(base_foreign_revid[1], base_foreign_revid[2])
                 bp = determine_branch_path(rev, layout, target_project)
                 target_config = self._get_branch_config(bp)
                 push_merged = (layout.push_merged_revisions(target_project) and
 
=== modified file 'remote.py'
--- remote.py	2011-07-31 17:33:06 +0000
+++ remote.py	2011-08-02 09:16:13 +0000
@@ -342,7 +342,7 @@
                 inter = InterToSvnRepository(source.repository, repos)
                 layout = repos.get_layout()
                 try:
-                    (type, project, _, ip) = layout.parse(target_branch_path)
+                    (type, project, _, ip) = layout.parse(target_branch_path, stop_revision)
                 except NotSvnBranchPath:
                     raise errors.NotBranchError(target_branch_path)
                 if type not in ('branch', 'tag') or ip != '':
 
=== modified file 'repository.py'
--- repository.py	2011-07-31 17:27:19 +0000
+++ repository.py	2011-08-02 09:16:13 +0000
@@ -983,7 +983,7 @@
             layout = self.get_layout()
         if foreign_sibling is not None and project is None:
             try:
-                project = layout.parse(foreign_sibling[1])[1]
+                project = layout.parse(foreign_sibling[1], foreign_sibling[2])[1]
             except errors.NotSvnBranchPath:
                 project = None
         return self.revmap.get_branch_revnum(revid, layout, project)
 
=== modified file 'tests/layout/test_standard.py'
--- tests/layout/test_standard.py	2011-07-26 14:18:15 +0000
+++ tests/layout/test_standard.py	2011-08-02 09:16:13 +0000
@@ -92,23 +92,23 @@
 
     def test_parse_trunk(self):
         self.assertEquals(("branch", "", "trunk", ""), 
-                          self.layout.parse("trunk"))
+                          self.layout.parse("trunk", None))
 
     def test_parse_trunk_tags(self):
         self.assertEquals(("branch", "", "trunk", "tags"), 
-                          self.layout.parse("trunk/tags"))
+                          self.layout.parse("trunk/tags", None))
 
     def test_parse_tag(self):
         self.assertEquals(("tag", "", "tags/foo", ""), 
-                          self.layout.parse("tags/foo"))
+                          self.layout.parse("tags/foo", None))
 
     def test_parse_branch(self):
         self.assertEquals(("branch", "", "branches/foo", ""), 
-                          self.layout.parse("branches/foo"))
+                          self.layout.parse("branches/foo", None))
 
     def test_parse_branch_project(self):
         self.assertEquals(("branch", "bla", "bla/branches/foo", ""), 
-                          self.layout.parse("bla/branches/foo"))
+                          self.layout.parse("bla/branches/foo", None))
 
     def test_parse_branches(self):
         self.assertRaises(NotSvnBranchPath, self.layout.parse, "branches")