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

Proposed by Yonggang Luo on 2011-08-02
Status: Work in progress
Proposed branch: lp:~luoyonggang/bzr-svn/parse_revnum
Merge into: lp:bzr-svn/1.1
Diff against target: 237 lines (+26/-26)
10 files modified
branch.py (+1/-1)
layout/__init__.py (+7/-7)
layout/standard.py (+5/-5)
mapping2.py (+2/-2)
mapping3/base.py (+1/-1)
metagraph.py (+1/-1)
push.py (+2/-2)
remote.py (+1/-1)
repository.py (+1/-1)
tests/layout/test_standard.py (+5/-5)
To merge this branch: bzr merge lp:~luoyonggang/bzr-svn/parse_revnum
Reviewer Review Type Date Requested Status
Jelmer Vernooij 2011-08-02 Pending
Review via email: mp+70126@code.launchpad.net

This proposal supersedes a proposal from 2011-08-02.

Description of the change

Add revnum parameters on some functions.

To post a comment you must log in.
Jelmer Vernooij (jelmer) wrote : Posted in a previous version of this proposal

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) wrote : Posted in a previous version of this proposal

>
> 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) wrote : Posted in a previous version of this proposal

which function?

Yonggang Luo (luoyonggang) wrote :

Ok

Jelmer Vernooij (jelmer) wrote : Posted in a previous version of this proposal

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) wrote : Posted in a previous version of this proposal

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) wrote : Posted in a previous version of this proposal

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) wrote : Posted in a previous version of this proposal

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

Jelmer Vernooij (jelmer) wrote :

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

Yonggang Luo (luoyonggang) wrote :

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) wrote :

> 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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
226 raise NotBranchError(self.base)226 raise NotBranchError(self.base)
227 raise227 raise
228 try:228 try:
229 (type, self.project, _, ip) = self.layout.parse(branch_path)229 (type, self.project, _, ip) = self.layout.parse(branch_path, revnum)
230 except NotSvnBranchPath:230 except NotSvnBranchPath:
231 raise NotBranchError(branch_path)231 raise NotBranchError(branch_path)
232 if type not in ('branch', 'tag') or ip != '':232 if type not in ('branch', 'tag') or ip != '':
233233
=== 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 @@
80 """80 """
81 raise NoCustomBranchPaths(self)81 raise NoCustomBranchPaths(self)
8282
83 def parse(self, path):83 def parse(self, path, revnum):
84 """Parse a path.84 """Parse a path with revnum.
8585
86 :return: Tuple with type ('tag', 'branch'), project name, branch path and path86 :return: Tuple with type ('tag', 'branch'), project name, branch path and path
87 inside the branch87 inside the branch
88 """88 """
89 raise NotImplementedError89 raise NotImplementedError
9090
91 def split_project_path(self, path, project):91 def split_project_path(self, path, project, revnum):
92 """Parse a project inside a particular project.92 """Parse a project inside a particular project at a particular revision.
9393
94 """94 """
95 (pt, parsed_project, bp, ip) = self.parse(path)95 (pt, parsed_project, bp, ip) = self.parse(path, revnum)
96 if project is not None and parsed_project != project:96 if project is not None and parsed_project != project:
97 raise NotSvnBranchPath(path, self)97 raise NotSvnBranchPath(path, self)
98 return (pt, bp, ip)98 return (pt, bp, ip)
@@ -100,7 +100,7 @@
100 def is_branch(self, path, project=None):100 def is_branch(self, path, project=None):
101 """Check whether a specified path points at a branch."""101 """Check whether a specified path points at a branch."""
102 try:102 try:
103 (type, proj, bp, rp) = self.parse(path)103 (type, proj, bp, rp) = self.parse(path, None)
104 except NotSvnBranchPath:104 except NotSvnBranchPath:
105 return False105 return False
106 if (type == "branch" and rp == "" and106 if (type == "branch" and rp == "" and
@@ -111,7 +111,7 @@
111 def is_tag(self, path, project=None):111 def is_tag(self, path, project=None):
112 """Check whether a specified path points at a tag."""112 """Check whether a specified path points at a tag."""
113 try:113 try:
114 (type, proj, bp, rp) = self.parse(path)114 (type, proj, bp, rp) = self.parse(path, None)
115 except NotSvnBranchPath:115 except NotSvnBranchPath:
116 return False116 return False
117 if (type == "tag" and rp == "" and117 if (type == "tag" and rp == "" and
118118
=== 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 @@
83 else:83 else:
84 return urlutils.join(project, "branches", name).strip("/")84 return urlutils.join(project, "branches", name).strip("/")
8585
86 def parse(self, path):86 def parse(self, path, revnum):
87 """Parse a path.87 """Parse a path.
8888
89 :return: Tuple with type ('tag', 'branch'), project name, branch path and path89 :return: Tuple with type ('tag', 'branch'), project name, branch path and path
@@ -190,7 +190,7 @@
190 raise svn_errors.NoCustomBranchPaths(self)190 raise svn_errors.NoCustomBranchPaths(self)
191 return ""191 return ""
192192
193 def parse(self, path):193 def parse(self, path, revnum):
194 """Parse a path.194 """Parse a path.
195195
196 :return: Tuple with type ('tag', 'branch'), project name, branch path and path196 :return: Tuple with type ('tag', 'branch'), project name, branch path and path
@@ -250,7 +250,7 @@
250 """250 """
251 return None251 return None
252252
253 def parse(self, path):253 def parse(self, path, revnum):
254 """Parse a path.254 """Parse a path.
255255
256 :return: Tuple with type ('tag', 'branch'), project name, branch path and path256 :return: Tuple with type ('tag', 'branch'), project name, branch path and path
@@ -367,7 +367,7 @@
367 return True367 return True
368 return False368 return False
369369
370 def parse(self, path):370 def parse(self, path, revnum):
371 """Parse a path.371 """Parse a path.
372372
373 :return: Tuple with type ('tag', 'branch'), project name, branch path373 :return: Tuple with type ('tag', 'branch'), project name, branch path
@@ -436,7 +436,7 @@
436 """436 """
437 return urlutils.join("branches", project, name).strip("/")437 return urlutils.join("branches", project, name).strip("/")
438438
439 def parse(self, path):439 def parse(self, path, revnum):
440 """Parse a path.440 """Parse a path.
441441
442 :return: Tuple with type ('tag', 'branch'), project name, branch path and path442 :return: Tuple with type ('tag', 'branch'), project name, branch path and path
443443
=== 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 @@
206 super(TrunkLegacyLayout, self).__init__()206 super(TrunkLegacyLayout, self).__init__()
207 self.level = level207 self.level = level
208208
209 def parse(self, path):209 def parse(self, path, revnum):
210 parts = path.strip("/").split("/")210 parts = path.strip("/").split("/")
211 if len(parts) == 0 or self.level >= len(parts):211 if len(parts) == 0 or self.level >= len(parts):
212 raise NotSvnBranchPath(path, self)212 raise NotSvnBranchPath(path, self)
@@ -247,7 +247,7 @@
247247
248class RootLegacyLayout(LegacyLayout):248class RootLegacyLayout(LegacyLayout):
249249
250 def parse(self, path):250 def parse(self, path, revnum):
251 return ("branch", "", "", path)251 return ("branch", "", "", path)
252252
253 def is_branch(self, path, project=None):253 def is_branch(self, path, project=None):
254254
=== 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 @@
54 self.repository = repository54 self.repository = repository
55 self.scheme = scheme55 self.scheme = scheme
5656
57 def parse(self, path):57 def parse(self, path, revnum):
58 try:58 try:
59 (proj, bp, rp) = self.scheme.unprefix(path)59 (proj, bp, rp) = self.scheme.unprefix(path)
60 except InvalidSvnBranchPath, e:60 except InvalidSvnBranchPath, e:
6161
=== 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 @@
441 continue441 continue
442 action = paths[p][0]442 action = paths[p][0]
443 try:443 try:
444 (_, bp, ip) = self.layout.split_project_path(p, self._project)444 (_, bp, ip) = self.layout.split_project_path(p, self._project, revnum)
445 except svn_errors.NotSvnBranchPath:445 except svn_errors.NotSvnBranchPath:
446 pass446 pass
447 else:447 else:
448448
=== 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 @@
480 if base_foreign_revid is None:480 if base_foreign_revid is None:
481 target_project = None481 target_project = None
482 else:482 else:
483 (_, target_project, _, _) = layout.parse(base_foreign_revid[1])483 (_, target_project, _, _) = layout.parse(base_foreign_revid[1], base_foreign_revid[2])
484 bp = determine_branch_path(rev, layout, target_project)484 bp = determine_branch_path(rev, layout, target_project)
485 target_config = self._get_branch_config(bp)485 target_config = self._get_branch_config(bp)
486 push_merged = (layout.push_merged_revisions(target_project) and486 push_merged = (layout.push_merged_revisions(target_project) and
@@ -596,7 +596,7 @@
596 if base_foreign_revid is None:596 if base_foreign_revid is None:
597 target_project = None597 target_project = None
598 else:598 else:
599 (_, target_project, _, _) = layout.parse(base_foreign_revid[1])599 (_, target_project, _, _) = layout.parse(base_foreign_revid[1], base_foreign_revid[2])
600 bp = determine_branch_path(rev, layout, target_project)600 bp = determine_branch_path(rev, layout, target_project)
601 target_config = self._get_branch_config(bp)601 target_config = self._get_branch_config(bp)
602 push_merged = (layout.push_merged_revisions(target_project) and602 push_merged = (layout.push_merged_revisions(target_project) and
603603
=== 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 @@
342 inter = InterToSvnRepository(source.repository, repos)342 inter = InterToSvnRepository(source.repository, repos)
343 layout = repos.get_layout()343 layout = repos.get_layout()
344 try:344 try:
345 (type, project, _, ip) = layout.parse(target_branch_path)345 (type, project, _, ip) = layout.parse(target_branch_path, stop_revision)
346 except NotSvnBranchPath:346 except NotSvnBranchPath:
347 raise errors.NotBranchError(target_branch_path)347 raise errors.NotBranchError(target_branch_path)
348 if type not in ('branch', 'tag') or ip != '':348 if type not in ('branch', 'tag') or ip != '':
349349
=== 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 @@
983 layout = self.get_layout()983 layout = self.get_layout()
984 if foreign_sibling is not None and project is None:984 if foreign_sibling is not None and project is None:
985 try:985 try:
986 project = layout.parse(foreign_sibling[1])[1]986 project = layout.parse(foreign_sibling[1], foreign_sibling[2])[1]
987 except errors.NotSvnBranchPath:987 except errors.NotSvnBranchPath:
988 project = None988 project = None
989 return self.revmap.get_branch_revnum(revid, layout, project)989 return self.revmap.get_branch_revnum(revid, layout, project)
990990
=== 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 @@
9292
93 def test_parse_trunk(self):93 def test_parse_trunk(self):
94 self.assertEquals(("branch", "", "trunk", ""), 94 self.assertEquals(("branch", "", "trunk", ""),
95 self.layout.parse("trunk"))95 self.layout.parse("trunk", None))
9696
97 def test_parse_trunk_tags(self):97 def test_parse_trunk_tags(self):
98 self.assertEquals(("branch", "", "trunk", "tags"), 98 self.assertEquals(("branch", "", "trunk", "tags"),
99 self.layout.parse("trunk/tags"))99 self.layout.parse("trunk/tags", None))
100100
101 def test_parse_tag(self):101 def test_parse_tag(self):
102 self.assertEquals(("tag", "", "tags/foo", ""), 102 self.assertEquals(("tag", "", "tags/foo", ""),
103 self.layout.parse("tags/foo"))103 self.layout.parse("tags/foo", None))
104104
105 def test_parse_branch(self):105 def test_parse_branch(self):
106 self.assertEquals(("branch", "", "branches/foo", ""), 106 self.assertEquals(("branch", "", "branches/foo", ""),
107 self.layout.parse("branches/foo"))107 self.layout.parse("branches/foo", None))
108108
109 def test_parse_branch_project(self):109 def test_parse_branch_project(self):
110 self.assertEquals(("branch", "bla", "bla/branches/foo", ""), 110 self.assertEquals(("branch", "bla", "bla/branches/foo", ""),
111 self.layout.parse("bla/branches/foo"))111 self.layout.parse("bla/branches/foo", None))
112112
113 def test_parse_branches(self):113 def test_parse_branches(self):
114 self.assertRaises(NotSvnBranchPath, self.layout.parse, "branches")114 self.assertRaises(NotSvnBranchPath, self.layout.parse, "branches")

Subscribers

People subscribed via source and target branches