Bazaar PQM Plugin

Merge lp:~wgrant/bzr-pqm/bug-711016-no-ui-clause into lp:bzr-pqm

  1. bug-711016-no-ui-clause
  2. Merge into devel
Proposed by William Grant
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: 77
Merged at revision: 77
Proposed branch: lp:~wgrant/bzr-pqm/bug-711016-no-ui-clause
Merge into: lp:bzr-pqm
Diff against target: 143 lines (+18/-18)
2 files modified
lpland.py (+3/-3)
tests/test_lpland.py (+15/-15)
To merge this branch: bzr merge lp:~wgrant/bzr-pqm/bug-711016-no-ui-clause
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) Approve
Review via email: mp+48135@code.launchpad.net

Description of the change

This branch adjusts lp-land to only include a [ui=...] clause when there are UI reviewers, rather than always including [ui=none]. This same change was made to ec2 land in https://code.launchpad.net/~wgrant/launchpad/bug-711016-no-ui-clause/+merge/48094.

To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Thanks.

As a side-note, I'm surprised there is this much project-specific code in bzr-pqm.

review: Approve
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

If I should land this, please let me know.

Revision history for this message
Aaron Bentley (abentley) wrote :

About the project-specific code in lp-land-- that's because I originally wrote lp-land as a standalone plugin for use with Launchpad landings. I believe that we are one of the few users of PQM, so it seemed fine to leave in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lpland.py'
2--- lpland.py 2010-11-08 16:49:24 +0000
3+++ lpland.py 2011-02-01 10:38:27 +0000
4@@ -386,13 +386,13 @@
5 if not code_reviewers:
6 raise MissingReviewError("Need approved votes in order to land.")
7 if ui_reviewers:
8- ui_clause = _comma_separated_names(ui_reviewers)
9+ ui_clause = '[ui=%s]' % _comma_separated_names(ui_reviewers)
10 else:
11- ui_clause = 'none'
12+ ui_clause = ''
13 if rc_reviewers:
14 rc_clause = (
15 '[release-critical=%s]' % _comma_separated_names(rc_reviewers))
16 else:
17 rc_clause = ''
18- return '%s[r=%s][ui=%s]' % (
19+ return '%s[r=%s]%s' % (
20 rc_clause, _comma_separated_names(code_reviewers), ui_clause)
21
22=== modified file 'tests/test_lpland.py'
23--- tests/test_lpland.py 2010-11-08 16:49:24 +0000
24+++ tests/test_lpland.py 2011-02-01 10:38:27 +0000
25@@ -209,7 +209,7 @@
26 self.mp.get_bugs = FakeMethod([self.fake_bug])
27 self.mp.get_reviews = FakeMethod({None : [self.fake_person]})
28
29- self.assertEqual("[r=foo][ui=none][bug=20] Foobaring the sbrubble.",
30+ self.assertEqual("[r=foo][bug=20] Foobaring the sbrubble.",
31 self.mp.build_commit_message("Foobaring the sbrubble.",
32 testfix, no_qa, incr))
33
34@@ -232,7 +232,7 @@
35 self.mp.get_bugs = FakeMethod([])
36 self.mp.get_reviews = FakeMethod({None : [self.fake_person]})
37
38- self.assertEqual("[r=foo][ui=none][no-qa] Foobaring the sbrubble.",
39+ self.assertEqual("[r=foo][no-qa] Foobaring the sbrubble.",
40 self.mp.build_commit_message("Foobaring the sbrubble.",
41 testfix, no_qa, incr))
42
43@@ -245,7 +245,7 @@
44 self.mp.get_reviews = FakeMethod({None : [self.fake_person]})
45
46 self.assertEqual(
47- "[r=foo][ui=none][bug=20][no-qa] Foobaring the sbrubble.",
48+ "[r=foo][bug=20][no-qa] Foobaring the sbrubble.",
49 self.mp.build_commit_message("Foobaring the sbrubble.",
50 testfix, no_qa, incr))
51
52@@ -258,7 +258,7 @@
53 self.mp.get_reviews = FakeMethod({None : [self.fake_person]})
54
55 self.assertEqual(
56- "[r=foo][ui=none][bug=20][incr] Foobaring the sbrubble.",
57+ "[r=foo][bug=20][incr] Foobaring the sbrubble.",
58 self.mp.build_commit_message("Foobaring the sbrubble.",
59 testfix, no_qa, incr))
60
61@@ -271,7 +271,7 @@
62 self.mp.get_reviews = FakeMethod({None : [self.fake_person]})
63
64 self.assertEqual(
65- "[r=foo][ui=none][bug=20][incr] Foobaring the sbrubble.",
66+ "[r=foo][bug=20][incr] Foobaring the sbrubble.",
67 self.mp.build_commit_message("Foobaring the sbrubble.",
68 testfix, no_qa, incr))
69
70@@ -284,7 +284,7 @@
71 self.mp.get_reviews = FakeMethod({None : [self.fake_person]})
72
73 self.assertEqual(
74- "[r=foo][ui=none][bug=20][no-qa][incr] Foobaring the sbrubble.",
75+ "[r=foo][bug=20][no-qa][incr] Foobaring the sbrubble.",
76 self.mp.build_commit_message("Foobaring the sbrubble.",
77 testfix, no_qa, incr))
78
79@@ -293,7 +293,7 @@
80 self.mp.get_reviews = FakeMethod({None : [self.fake_person]})
81
82 self.assertEqual(
83- "[r=foo][ui=none][bug=20][rollback=123] Foobaring the sbrubble.",
84+ "[r=foo][bug=20][rollback=123] Foobaring the sbrubble.",
85 self.mp.build_commit_message("Foobaring the sbrubble.",
86 rollback=123))
87
88@@ -302,9 +302,9 @@
89 self.mp.get_reviews = FakeMethod({None : [self.fake_person]})
90
91 self.assertEqual(
92- "[r=foo][ui=none][bug=20][rollback=123] Foobaring the sbrubble.",
93+ "[r=foo][bug=20][rollback=123] Foobaring the sbrubble.",
94 self.mp.build_commit_message(
95- "[r=foo][ui=none][bug=20][rollback=123] Foobaring the sbrubble.",
96+ "[r=foo][bug=20][rollback=123] Foobaring the sbrubble.",
97 rollback=123))
98
99
100@@ -331,15 +331,15 @@
101 def test_one_reviewer_no_type(self):
102 # It's very common for a merge proposal to be reviewed by one person
103 # with no specified type of review. It such cases the review clause is
104- # '[r=<person>][ui=none]'.
105+ # '[r=<person>]'.
106 clause = self.get_reviewer_clause({None: [self.makePerson('foo')]})
107- self.assertEqual('[r=foo][ui=none]', clause)
108+ self.assertEqual('[r=foo]', clause)
109
110 def test_two_reviewers_no_type(self):
111 # Branches can have more than one reviewer.
112 clause = self.get_reviewer_clause(
113 {None: [self.makePerson('foo'), self.makePerson('bar')]})
114- self.assertEqual('[r=bar,foo][ui=none]', clause)
115+ self.assertEqual('[r=bar,foo]', clause)
116
117 def test_mentat_reviewers(self):
118 # A mentat review sometimes is marked like 'ui*'. Due to the
119@@ -357,7 +357,7 @@
120 # reviews, these are treated in the same way as reviewers without any
121 # given type.
122 clause = self.get_reviewer_clause({'code': [self.makePerson('foo')]})
123- self.assertEqual('[r=foo][ui=none]', clause)
124+ self.assertEqual('[r=foo]', clause)
125
126 def test_release_critical(self):
127 # Reviews that are marked as release-critical are included in a
128@@ -365,13 +365,13 @@
129 clause = self.get_reviewer_clause(
130 {'code': [self.makePerson('foo')],
131 'release-critical': [self.makePerson('bar')]})
132- self.assertEqual('[release-critical=bar][r=foo][ui=none]', clause)
133+ self.assertEqual('[release-critical=bar][r=foo]', clause)
134
135 def test_db_reviewer_counts(self):
136 # There's no special way of annotating database reviews in Launchpad
137 # commit messages, so they are included with the code reviews.
138 clause = self.get_reviewer_clause({'db': [self.makePerson('foo')]})
139- self.assertEqual('[r=foo][ui=none]', clause)
140+ self.assertEqual('[r=foo]', clause)
141
142 def test_ui_reviewers(self):
143 # If someone has done a UI review, then that appears in the clause

Subscribers

People subscribed via source and target branches

to all changes:
to status/vote changes: