launchpad-dev team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

On Thu, Dec 1, 2011 at 4:16 AM, Aaron Bentley <aaron@xxxxxxxxxxxxx> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 11-11-29 05:14 PM, Robert Collins wrote:
>> e.g. (if lifeless is my object id) 'lifeless is subscribed to
>> event tag 'assignee:lifeless'
>
> I was thinking of the narrow case of adding and removing
> relationships, so you'd have
>
> notify("$USER requested to review", "$USER requested to review",
> "$USER requested to review", ['merge-proposal', 'add-relationship',
> 'add-reviewer'], ['BranchMergeProposal:25', 'Person:abentley'], [])
>
> I don't really understand how your elaboration would work.  ISTM that
> if all reviewers were listed as event tags, it would be hard to work
> out whose relationship had changed:

Interesting case. Clearly existing reviewers who are not subscribed in
any other way still deserve to be notified that the MP has changed.
And the new reviewer needs a copy of the current diff + whatever else
we think is relevant to bring them up to speed.

That sounds like two events to me:
 - requested review with one participant
 - merge proposal has a new reviewer with either all reviewers as
participants + event collapsing, or all previously existing reviewers
as participants.

Separately it occurs to me we may want to take a web microformat
approach and incorporate a standard subject-verb-object model :
lifeless requested-review-from-abently-on branch-X ... or something
like that. Branch-X as the object would let us use that for event
collapsing I think. OTOH topics are objects. o/ more synthesis needed.

> notify("$USER requested to review", "$USER requested to review",
> "$USER requested to review", ['merge-proposal', 'add-relationship',
> 'add-reviewer', 'reviewer:abentley', 'reviewer:lifeless'],
> ['BranchMergeProposal:25'], [])
>
> btw, I think topics might be better as a mapping, so that we can
> distinguish between two objects of the same type with different roles.
>
> notify("proposal created", "proposal created", "proposal
> created",['merge-proposal', 'add-relationship', 'add-reviewer'],
> {"source_branch": 'Branch:12', "target_branch": 'Branch:10',
> "proposal": "BranchMergeProposal:25"}, [])

Making topics a mapping would be fine I think, depending on what we
want from the keys.
Consider that you can say 'source-branch:Branch:12' as effectively as
'source_branch':'Branch:12' - I don't think it changes the
capabilities; it may change the query logic though.

I was thinking we'd emit multiple events for a new merge proposal though:
 - one for the merge proposal
 - one for the review-requested case

because we want to say 'review requested' to folk that are going to be
reviewers and yet we have other folk (and bots etc) that aren't
reviewers but want to know that the MP has been created.

Perhaps the question is 'when there are multiple different events that
may notify one person, about one object, how should we combine them?'
What we do for bugs today is to create a single message by combining
all the individual events.

This runs into a few problems, one of which is how to combine the
events cleanly (we do ok on this today, for bugs) and another is that
we don't know how to combine *reasons for notification* - if you are
subscribed to a bug comments only, and someone comments on it and then
assigns you, you are strictly being notified for two separate reasons,
for the two separate events - but we probably want to combine them.

So, lets assume that this is a critical and important facility; we
need an algorithm to combine event templates sensibly, or a different
rendering model. And we need an algorithm to decide when two separate
notifications should be merged (in the web UI, in email etc).

One simple heuristic for when to combine is 'if it is about the same
object'. E.g. if you have three notifications - one for 'new merge
into branch A' one for 'new merge from branch B' and one for 'you are
requested to review this new merge' - we can see they all have the new
merge in common, so after filtering for your subscriptions, even if
all three are events you are subscribed to, we collapse
email/direct-tweet/sms notifications down to one. In a timeline view
of any of those direct objects, we can show just the direct events. In
a timeline view of e.g. the project, we can apply the same collapsing
logic as for email/direct-tweet/sms and get just the merge proposal.

So coming right around to reviewers:
notify('review requested for branch $source into $target', ..., ...
['review-requested', 'reviewer:Person:launchpad-reviewers', 'code',
'review'], ['MergeProposal:1234', 'source:Branch:45',
'target:Branch:48', 'Project:56'], ['state:needs-review'])
notify('reviewer added', ..., ... ['reviewer-added',
'reviewer:Person:launchpad-reviewers', 'code', 'review'],
['MergeProposal:1234', 'source:Branch:45', 'target:Branch:48',
'Project:56'], ['state:needs-review'])

We'd subscribe branch reviewers to 'review-requested' && Branch:48
which won't match the second notification.
Branch MP subscribers we'd subscribe to Branch:48 &&
-review-requested. [Treating the review-requested tag as one which you
must explicitly subscribe to to get messages from.
Branch revision subscribers would subscribe to Branch:48 && -review.
[So getting only non-review related notifications].

Still leaves open a question on how to combine the reviewer-added and
review-requested-from-you notifications.

> It seems like it would be nice if we didn't have to specify both
> summary and subject.

So I am thinking of the following situations:
 - one liners (twitter) / in-app timelines
 - few-liners (RSS)
 - comprehensive (email)

It seems better to me that we decide how to compact things ourselves
rather than getting textual jaggies :)

> Also, what are subject_template, body_template and summary_template?
> HTML?  MIME?

Probably markdown or rest - something we can render to text and render
to html. Possibly we don't want templates but just literal markdown
etc to output. The idea around templates is for personalisation on
subscriber expansions.

-Rob