← Back to team overview

launchpad-reviewers team mailing list archive

Re: lp:~danilo/launchpad/bug-772754-other-subscribers-remove-cruft into lp:launchpad

 

Hi Gary, thanks for looking at this branch.

The first one should have already been fixed (there are even tests to prove it, but there was a bug in the ordering logic that allowed it to sometimes be misordered; both tests and code have been improved to catch this case). If it's not in this branch yet, sorry about it (that's one thing I wish pipelines made easier: pushing _all_ branches up).

As for the second, I omitted it on purpose: long text tends to not be read.  As far as the help link, I'd like that, but I'll have to get help from Matt (or someone else) on the text, so I am leaving that for a next branch.

As for the third thing, the code is structured in a way that it should be easy to add another "action" (of which unsubscribe is only one option offered atm) which allows one to change the level of the subscription, but since we don't have that for teams yet anyway, I didn't want to implement it here. IOW, adding the icon/action should be easy, making it do the right thing is out of scope.


-- 
https://code.launchpad.net/~danilo/launchpad/bug-772754-other-subscribers-remove-cruft/+merge/64188
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/bug-772754-other-subscribers-remove-cruft into lp:launchpad.


References