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
1=== modified file 'branch.py'
2--- branch.py 2011-07-21 13:42:21 +0000
3+++ branch.py 2011-08-02 09:16:13 +0000
4@@ -226,7 +226,7 @@
5 raise NotBranchError(self.base)
6 raise
7 try:
8- (type, self.project, _, ip) = self.layout.parse(branch_path)
9+ (type, self.project, _, ip) = self.layout.parse(branch_path, revnum)
10 except NotSvnBranchPath:
11 raise NotBranchError(branch_path)
12 if type not in ('branch', 'tag') or ip != '':
13
14=== modified file 'layout/__init__.py'
15--- layout/__init__.py 2011-06-29 15:45:06 +0000
16+++ layout/__init__.py 2011-08-02 09:16:13 +0000
17@@ -80,19 +80,19 @@
18 """
19 raise NoCustomBranchPaths(self)
20
21- def parse(self, path):
22- """Parse a path.
23+ def parse(self, path, revnum):
24+ """Parse a path with revnum.
25
26 :return: Tuple with type ('tag', 'branch'), project name, branch path and path
27 inside the branch
28 """
29 raise NotImplementedError
30
31- def split_project_path(self, path, project):
32- """Parse a project inside a particular project.
33+ def split_project_path(self, path, project, revnum):
34+ """Parse a project inside a particular project at a particular revision.
35
36 """
37- (pt, parsed_project, bp, ip) = self.parse(path)
38+ (pt, parsed_project, bp, ip) = self.parse(path, revnum)
39 if project is not None and parsed_project != project:
40 raise NotSvnBranchPath(path, self)
41 return (pt, bp, ip)
42@@ -100,7 +100,7 @@
43 def is_branch(self, path, project=None):
44 """Check whether a specified path points at a branch."""
45 try:
46- (type, proj, bp, rp) = self.parse(path)
47+ (type, proj, bp, rp) = self.parse(path, None)
48 except NotSvnBranchPath:
49 return False
50 if (type == "branch" and rp == "" and
51@@ -111,7 +111,7 @@
52 def is_tag(self, path, project=None):
53 """Check whether a specified path points at a tag."""
54 try:
55- (type, proj, bp, rp) = self.parse(path)
56+ (type, proj, bp, rp) = self.parse(path, None)
57 except NotSvnBranchPath:
58 return False
59 if (type == "tag" and rp == "" and
60
61=== modified file 'layout/standard.py'
62--- layout/standard.py 2011-07-26 14:18:15 +0000
63+++ layout/standard.py 2011-08-02 09:16:13 +0000
64@@ -83,7 +83,7 @@
65 else:
66 return urlutils.join(project, "branches", name).strip("/")
67
68- def parse(self, path):
69+ def parse(self, path, revnum):
70 """Parse a path.
71
72 :return: Tuple with type ('tag', 'branch'), project name, branch path and path
73@@ -190,7 +190,7 @@
74 raise svn_errors.NoCustomBranchPaths(self)
75 return ""
76
77- def parse(self, path):
78+ def parse(self, path, revnum):
79 """Parse a path.
80
81 :return: Tuple with type ('tag', 'branch'), project name, branch path and path
82@@ -250,7 +250,7 @@
83 """
84 return None
85
86- def parse(self, path):
87+ def parse(self, path, revnum):
88 """Parse a path.
89
90 :return: Tuple with type ('tag', 'branch'), project name, branch path and path
91@@ -367,7 +367,7 @@
92 return True
93 return False
94
95- def parse(self, path):
96+ def parse(self, path, revnum):
97 """Parse a path.
98
99 :return: Tuple with type ('tag', 'branch'), project name, branch path
100@@ -436,7 +436,7 @@
101 """
102 return urlutils.join("branches", project, name).strip("/")
103
104- def parse(self, path):
105+ def parse(self, path, revnum):
106 """Parse a path.
107
108 :return: Tuple with type ('tag', 'branch'), project name, branch path and path
109
110=== modified file 'mapping2.py'
111--- mapping2.py 2011-07-26 14:18:15 +0000
112+++ mapping2.py 2011-08-02 09:16:13 +0000
113@@ -206,7 +206,7 @@
114 super(TrunkLegacyLayout, self).__init__()
115 self.level = level
116
117- def parse(self, path):
118+ def parse(self, path, revnum):
119 parts = path.strip("/").split("/")
120 if len(parts) == 0 or self.level >= len(parts):
121 raise NotSvnBranchPath(path, self)
122@@ -247,7 +247,7 @@
123
124 class RootLegacyLayout(LegacyLayout):
125
126- def parse(self, path):
127+ def parse(self, path, revnum):
128 return ("branch", "", "", path)
129
130 def is_branch(self, path, project=None):
131
132=== modified file 'mapping3/base.py'
133--- mapping3/base.py 2011-07-26 14:18:15 +0000
134+++ mapping3/base.py 2011-08-02 09:16:13 +0000
135@@ -54,7 +54,7 @@
136 self.repository = repository
137 self.scheme = scheme
138
139- def parse(self, path):
140+ def parse(self, path, revnum):
141 try:
142 (proj, bp, rp) = self.scheme.unprefix(path)
143 except InvalidSvnBranchPath, e:
144
145=== modified file 'metagraph.py'
146--- metagraph.py 2011-07-16 10:05:48 +0000
147+++ metagraph.py 2011-08-02 09:16:13 +0000
148@@ -441,7 +441,7 @@
149 continue
150 action = paths[p][0]
151 try:
152- (_, bp, ip) = self.layout.split_project_path(p, self._project)
153+ (_, bp, ip) = self.layout.split_project_path(p, self._project, revnum)
154 except svn_errors.NotSvnBranchPath:
155 pass
156 else:
157
158=== modified file 'push.py'
159--- push.py 2011-07-26 09:57:59 +0000
160+++ push.py 2011-08-02 09:16:13 +0000
161@@ -480,7 +480,7 @@
162 if base_foreign_revid is None:
163 target_project = None
164 else:
165- (_, target_project, _, _) = layout.parse(base_foreign_revid[1])
166+ (_, target_project, _, _) = layout.parse(base_foreign_revid[1], base_foreign_revid[2])
167 bp = determine_branch_path(rev, layout, target_project)
168 target_config = self._get_branch_config(bp)
169 push_merged = (layout.push_merged_revisions(target_project) and
170@@ -596,7 +596,7 @@
171 if base_foreign_revid is None:
172 target_project = None
173 else:
174- (_, target_project, _, _) = layout.parse(base_foreign_revid[1])
175+ (_, target_project, _, _) = layout.parse(base_foreign_revid[1], base_foreign_revid[2])
176 bp = determine_branch_path(rev, layout, target_project)
177 target_config = self._get_branch_config(bp)
178 push_merged = (layout.push_merged_revisions(target_project) and
179
180=== modified file 'remote.py'
181--- remote.py 2011-07-31 17:33:06 +0000
182+++ remote.py 2011-08-02 09:16:13 +0000
183@@ -342,7 +342,7 @@
184 inter = InterToSvnRepository(source.repository, repos)
185 layout = repos.get_layout()
186 try:
187- (type, project, _, ip) = layout.parse(target_branch_path)
188+ (type, project, _, ip) = layout.parse(target_branch_path, stop_revision)
189 except NotSvnBranchPath:
190 raise errors.NotBranchError(target_branch_path)
191 if type not in ('branch', 'tag') or ip != '':
192
193=== modified file 'repository.py'
194--- repository.py 2011-07-31 17:27:19 +0000
195+++ repository.py 2011-08-02 09:16:13 +0000
196@@ -983,7 +983,7 @@
197 layout = self.get_layout()
198 if foreign_sibling is not None and project is None:
199 try:
200- project = layout.parse(foreign_sibling[1])[1]
201+ project = layout.parse(foreign_sibling[1], foreign_sibling[2])[1]
202 except errors.NotSvnBranchPath:
203 project = None
204 return self.revmap.get_branch_revnum(revid, layout, project)
205
206=== modified file 'tests/layout/test_standard.py'
207--- tests/layout/test_standard.py 2011-07-26 14:18:15 +0000
208+++ tests/layout/test_standard.py 2011-08-02 09:16:13 +0000
209@@ -92,23 +92,23 @@
210
211 def test_parse_trunk(self):
212 self.assertEquals(("branch", "", "trunk", ""),
213- self.layout.parse("trunk"))
214+ self.layout.parse("trunk", None))
215
216 def test_parse_trunk_tags(self):
217 self.assertEquals(("branch", "", "trunk", "tags"),
218- self.layout.parse("trunk/tags"))
219+ self.layout.parse("trunk/tags", None))
220
221 def test_parse_tag(self):
222 self.assertEquals(("tag", "", "tags/foo", ""),
223- self.layout.parse("tags/foo"))
224+ self.layout.parse("tags/foo", None))
225
226 def test_parse_branch(self):
227 self.assertEquals(("branch", "", "branches/foo", ""),
228- self.layout.parse("branches/foo"))
229+ self.layout.parse("branches/foo", None))
230
231 def test_parse_branch_project(self):
232 self.assertEquals(("branch", "bla", "bla/branches/foo", ""),
233- self.layout.parse("bla/branches/foo"))
234+ self.layout.parse("bla/branches/foo", None))
235
236 def test_parse_branches(self):
237 self.assertRaises(NotSvnBranchPath, self.layout.parse, "branches")

Subscribers

People subscribed via source and target branches