Nice branch! I've got a few comments but they're all cosmetic really. Otherwise, r=me.
[1]
62 +
63 + if self.simplified_branch_menu:
64 + return (
65 + self.registered_branch_not_empty or
66 + self.owned_branch_not_empty or
67 + self.subscribed_branch_not_empty or
68 + self.active_review_not_empty
69 + )
70 + else:
71 + return (self.owned_branch_count or
72 + self.registered_branch_count or
73 + self.subscribed_branch_count or
74 + self.active_review_count)
Slightly incosistent formatting here: The form you've used from
64-69 (newline after the opening parenthesis) is the correct one, though
the closing paren should go on the last line as you've done on line 74.
[2]
89 + @cachedproperty
90 + def owned_branch_not_empty(self):
91 + """False if the number of branches owned by self.person is zero."""
92 + return not self._getCountCollection().ownedBy(self.person).is_empty()
I get the feeling this should be named "owned_branches_not_empty" since
you're talking about a collection but the property name suggests you're
talking about a single branch.
By convention, if you're going to declare a class attribute, you should
do it at the top of the class, since otherwise the reader (i.e. your
humble bearded reviewer) gets a bit confused :).
Hi Raphaël,
Nice branch! I've got a few comments but they're all cosmetic really. Otherwise, r=me.
[1]
62 + _branch_ menu: _branch_ not_empty or branch_ not_empty or _branch_ not_empty or review_ not_empty branch_ count or _branch_ count or _branch_ count or review_ count)
63 + if self.simplified
64 + return (
65 + self.registered
66 + self.owned_
67 + self.subscribed
68 + self.active_
69 + )
70 + else:
71 + return (self.owned_
72 + self.registered
73 + self.subscribed
74 + self.active_
Slightly incosistent formatting here: The form you've used from
64-69 (newline after the opening parenthesis) is the correct one, though
the closing paren should go on the last line as you've done on line 74.
[2]
89 + @cachedproperty not_empty( self): ollection( ).ownedBy( self.person) .is_empty( )
90 + def owned_branch_
91 + """False if the number of branches owned by self.person is zero."""
92 + return not self._getCountC
I get the feeling this should be named "owned_ branches_ not_empty" since
you're talking about a collection but the property name suggests you're
talking about a single branch.
[3]
94 + @cachedproperty branch_ not_empty( self):
95 + def subscribed_
... And by the same token this should be subscribed_ branches_ not_empty.
[4]
103 + @cachedproperty review_ not_empty( self):
104 + def active_
active_ reviews_ not_empty
[5]
224 + registered_ branches_ matcher = soupmatchers. HTMLContains( launchpad. dev/~barney' anches' }))
225 + soupmatchers.Tag(
226 + 'Registered link', 'a', text='Registered branches',
227 + attrs={'href': 'http://
228 + '/+registeredbr
229 +
By convention, if you're going to declare a class attribute, you should
do it at the top of the class, since otherwise the reader (i.e. your
humble bearded reviewer) gets a bit confused :).