Code review comment for lp:~thumper/launchpad/branch-subscription-subscribed-by

> === modified file 'lib/lp/code/browser/branch.py'
> --- lib/lp/code/browser/branch.py 2010-05-20 04:01:34 +0000
> +++ lib/lp/code/browser/branch.py 2010-05-27 04:47:25 +0000
> @@ -354,6 +354,7 @@
>
> def initialize(self):
> self.notices = []
> + # TODO: FIXME - this is almost certainly not what we want.
> self._add_subscription_notice()

What do you want to do here?

> === modified file 'lib/lp/code/browser/branchsubscription.py'
> --- lib/lp/code/browser/branchsubscription.py 2010-04-20 01:21:10 +0000
> +++ lib/lp/code/browser/branchsubscription.py 2010-05-27 04:47:25 +0000
> ...
>
> @@ -209,7 +210,7 @@
> subscription = self.context.getSubscription(person)
> if subscription is None:
> self.context.subscribe(
> - person, notification_level, max_diff_lines, review_level)
> + person, notification_level, max_diff_lines, review_level, self.user)

Wrap the code at 78 characters.

> === modified file 'lib/lp/code/model/branchsubscription.py'
> --- lib/lp/code/model/branchsubscription.py 2009-06-25 04:06:00 +0000
> +++ lib/lp/code/model/branchsubscription.py 2010-05-27 04:47:25 +0000
> @@ -41,6 +41,9 @@
> notNull=False, default=DEFAULT)
> review_level = EnumCol(enum=CodeReviewNotificationLevel,
> notNull=True, default=DEFAULT)
> + subscribed_by = ForeignKey(
> + dbName='subscribed_by', foreignKey='Person',
> + storm_validator=validate_person_not_private_membership, notNull=True)

I was suggested in a recent review to drop the dbName since it is identical
to the column name. I did so, but now I ponder explicit is better than
implicit again.

> === modified file 'lib/lp/registry/browser/tests/packaging-views.txt'
> --- lib/lp/registry/browser/tests/packaging-views.txt 2010-04-16 18:00:31 +0000
> +++ lib/lp/registry/browser/tests/packaging-views.txt 2010-05-27 04:47:25 +0000
> @@ -350,5 +350,5 @@
> cnews
> libstdc++
> linux-source-2.6.15
> + hot
> thunderbird
> - hot

What caused this? This look like me from last week?

> === modified file 'lib/lp/registry/model/distroseries.py'
> --- lib/lp/registry/model/distroseries.py 2010-05-12 23:23:19 +0000
> +++ lib/lp/registry/model/distroseries.py 2010-05-27 04:47:25 +0000
> @@ -332,7 +332,7 @@
> origin = SQL(joins)
> condition = SQL(conditions + "AND packaging.id IS NULL")
> results = IStore(self).using(origin).find(find_spec, condition)
> - results = results.order_by('score DESC')
> + results = results.order_by('score DESC', SourcePackageName.name)
> return [{
> 'package': SourcePackage(
> sourcepackagename=spn, distroseries=self),

This is me from last week.

> === modified file 'lib/lp/testing/factory.py'
> --- lib/lp/testing/factory.py 2010-05-25 13:13:02 +0000
> +++ lib/lp/testing/factory.py 2010-05-27 04:47:25 +0000
> @@ -986,7 +986,7 @@
>
> return proposal
>
> - def makeBranchSubscription(self, branch=None, person=None):
> + def makeBranchSubscription(self, branch=None, person=None, subscribed_by=None):
> """Create a BranchSubscription.

Wrap the code at 78 characters.