Merge lp:~abentley/launchpad/mp-tweakage into lp:launchpad

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

 reviewer beuno ui

= Summary =
Tweaks to merge proposal main page

== Proposed fix ==
linked bugs and blueprints are pulled above the comments, so they are
easy to see.

"add comment or review" is pulled into the vote table, so it is easy to
use, and to strengthen the association with the requested reviews.

'resubmit' is provided as a direct link, and placed with the information
about whether this branch supersedes another branch.

== Pre-implementation notes ==
None

== Implementation details ==
I haven't removed the "resubmit" merge proposal status yet, but
providing resubmit as a link obsoletes it.

== Tests ==
None. I want to get the UI approved before I polish.

== Demo and Q/A ==
Create a merge proposal, and link a bug to the source branch. Go to the
main merge proposal page. See "Proposed fix" for what the page should
look like.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/code/templates/branchmergeproposal-index.pt
  lib/lp/code/browser/branchmergeproposal.py
  lib/lp/code/templates/branchmergeproposal-votes.pt
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpx7rcACgkQ0F+nu1YWqI3oJACePEgSs4FW87Kz16Sp1qZGFUXg
QMIAnR7FJdhZV+rjzcSjVzKMEPNxjWJN
=FyY9
-----END PGP SIGNATURE-----

Hi Aaron,

It's nice to see work on merge proposals.

I feel that going down the path of "Add a review or comment" has not been good. Reviewing and commenting are very different things, and mixing them just creates confusion.
I think we should clearly separate them into different links, and just distinguish the page it goes to by having the review section collapsed or expanded by default. That way we still allow people to do both, but lead them down two different paths.

I also think the way that the links are piled up make it hard to find them. When I have been requested to review, I have a [Review] link, and a "Add a review or comment" very close to it. Makes me think that, leaving aside the issue mentioned before, that link should at a very minimum be outside the table.

As for the link to resubmit the proposal, I feel that clicking a link to then click a button is not the best experience. Can't we just have the button (as a link!) on the page?
In the 3.0 spirit, this seems like something that would go on the global actions portlets we will have on the top-right. Since that template is not yet available, I think we should fix it's current placement, which feels slapped on randomly. I wonder if the right place is where we display commits? That area can be pushed down quite a bit, so I don't know.

In general, I think this branch doesn't improve the user experience, but the spirit and direction is good, so with some work it will be great.

On Thu, 2009-07-30 at 22:46 +0000, Martin Albisetti wrote:
>
> I feel that going down the path of "Add a review or comment" has not
> been good. Reviewing and commenting are very different things, and
> mixing them just creates confusion.
> I think we should clearly separate them into different links, and just
> distinguish the page it goes to by having the review section collapsed
> or expanded by default. That way we still allow people to do both, but
> lead them down two different paths.

Huh? I review by email, and I think that commenting and reviewing are
nearly identical. The only difference (to me) is that sometimes I don't
feel my opinion fits the launchpad set of values strongly or clearly
enough to express, and in those cases I don't give lp a specific
command.

-Rob

(Commenting, in this case :P)

> Huh? I review by email, and I think that commenting and reviewing are
> nearly identical. The only difference (to me) is that sometimes I don't
> feel my opinion fits the launchpad set of values strongly or clearly
> enough to express, and in those cases I don't give lp a specific
> command.

That's because you're a reviewer. As a reviewer, the line is blurry-er. As someone who just has an opinion, it's not.

On Thu, Jul 30, 2009 at 11:05:17PM -0000, Martin Albisetti wrote:
> > Huh? I review by email, and I think that commenting and reviewing are
> > nearly identical. The only difference (to me) is that sometimes I don't
> > feel my opinion fits the launchpad set of values strongly or clearly
> > enough to express, and in those cases I don't give lp a specific
> > command.
>
> That's because you're a reviewer. As a reviewer, the line is
> blurry-er. As someone who just has an opinion, it's not.

If we do go down the path of separating commenting and reviewing, please
add a way of subscribing to the merge proposal if you comment on it
only. At the moment you have to add yourself as a reviewer, meaning that
even this comment is a kind of review.

    vote abstain

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Albisetti wrote:

> I feel that going down the path of "Add a review or comment" has not been good. Reviewing and commenting are very different things, and mixing them just creates confusion.

Thanks for your review.

Can you explain what makes them different? To me, a review is just a
comment with a little bit of extra information. It forms part of a
discussion, which is why I want people to be able to perform a review in
reply to a comment.

Launchpad which needs us to record reviews, so that it can provide a
better summary. Humans get along fine without the Launchpad metadata,
though having it does improve clarity.

I think part of the confusion is that the dropdown says "-Select-",
implying the user must choose. It should say "Just a comment", "Not a
review", or something like that.

> I think we should clearly separate them into different links, and just distinguish the page it goes to by having the review section collapsed or expanded by default. That way we still allow people to do both, but lead them down two different paths.

I still feel that drawing such a strong distinction, and then showing it
to be unnecessary by allowing people to do either from either page
doesn't seem like a good approach to me.

I should mention that people who reply often don't know whether they're
providing a review or a comment until after they've finished typing it
up. They only know whether they can make a verdict until after they've
fully explored the issues through typing their response up, and become
certain there are no outstanding questions.

If we have separate links, then we should provide them in all the same
places. Everywhere it currently says "Reply", it would have a link for
"Reply as comment" and "Reply as review".

> I also think the way that the links are piled up make it hard to find them. When I have been requested to review, I have a [Review] link, and a "Add a review or comment" very close to it.

As you suggested recently, I'd like to have people choose which request
their review satisfies using a dropdown.

That would mean we could remove the [Review] links, which is what I plan
to do.

> Makes me think that, leaving aside the issue mentioned before, that link should at a very minimum be outside the table.

I can do that, but I deliberately placed it as close to the review
requests as I possibly could, so that when people see the request to
review, it's very easy to respond.

> As for the link to resubmit the proposal, I feel that clicking a link to then click a button is not the best experience. Can't we just have the button (as a link!) on the page?

Resubmitting is a painful process to reverse, so I think it makes sense
to require confirmation.

We should fix 383352, which would make the resubmit page more than
merely a confirmation page.

I understood that links which change state were against our policy.

> In the 3.0 spirit, this seems like something that would go on the global actions portlets we will have on the top-right. Since that template is not yet available, I think we should fix it's current placement, which feels slapped on randomly.

As I explained in the c...

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Albisetti wrote:
>> Huh? I review by email, and I think that commenting and reviewing are
>> nearly identical. The only difference (to me) is that sometimes I don't
>> feel my opinion fits the launchpad set of values strongly or clearly
>> enough to express, and in those cases I don't give lp a specific
>> command.
>
> That's because you're a reviewer.

But everyone is a reviewer.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpzAFwACgkQ0F+nu1YWqI3gYACeKrpFZWSVbRxX8EXrpc/W+Jku
SAIAniIXdGMcRSCPAjzSigcBG+jPxiCv
=mrvq
-----END PGP SIGNATURE-----

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Björn Tillenius wrote:
> If we do go down the path of separating commenting and reviewing, please
> add a way of subscribing to the merge proposal if you comment on it
> only.

Mark Shuttleworth has specifically forbidden code reviews from having
subscriptions. Please bring it up with him.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpzARgACgkQ0F+nu1YWqI2YAQCeIgWmzzw61/V2NopszOYd7PqJ
0ysAn1f5mZ1De9ItXg12ZKWvPvoFC0C1
=CTQF
-----END PGP SIGNATURE-----

On Fri, Jul 31, 2009 at 02:36:08PM -0000, Aaron Bentley wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Björn Tillenius wrote:
> > If we do go down the path of separating commenting and reviewing, please
> > add a way of subscribing to the merge proposal if you comment on it
> > only.
>
> Mark Shuttleworth has specifically forbidden code reviews from having
> subscriptions. Please bring it up with him.

It doesn't have to be to the merge proposal directly. I would love an
easy way of subscribing to the branch that is to be merged, so that I
would get both code review mail, and notifications about new revisions.

--
Björn Tillenius | https://launchpad.net/~bjornt

> > I feel that going down the path of "Add a review or comment" has not been
> good. Reviewing and commenting are very different things, and mixing them just
> creates confusion.
>
> Thanks for your review.
>
> Can you explain what makes them different? To me, a review is just a
> comment with a little bit of extra information. It forms part of a
> discussion, which is why I want people to be able to perform a review in
> reply to a comment.

I've been involved in many MPs where I just want to comment, not weigh in on the decision. Setting a status is a strong message, and a lot of people don't feel they have that kind of authority. Commenting helps to bring down the barrier of entry.

> I think part of the confusion is that the dropdown says "-Select-",
> implying the user must choose. It should say "Just a comment", "Not a
> review", or something like that.

Yes, I think this is part of the problem. Collapsing this section if you click on "Add a comment", and optionally allow you to expand may be a good middle-ground, but it may also end of feeling like "design by committee",

> > I think we should clearly separate them into different links, and just
> distinguish the page it goes to by having the review section collapsed or
> expanded by default. That way we still allow people to do both, but lead them
> down two different paths.
>
> I still feel that drawing such a strong distinction, and then showing it
> to be unnecessary by allowing people to do either from either page
> doesn't seem like a good approach to me.

Cramming both options in one link because we can feels very wrong.

> I should mention that people who reply often don't know whether they're
> providing a review or a comment until after they've finished typing it
> up. They only know whether they can make a verdict until after they've
> fully explored the issues through typing their response up, and become
> certain there are no outstanding questions.

Yes, this happens to me very often. Sometimes I really know (I click on the review link), sometimes I know I don't want to review (click on comment), and sometimes I don't know (I tendo to click on comment). Which is why I still want the option, but hide the review boxes in one.

> If we have separate links, then we should provide them in all the same
> places. Everywhere it currently says "Reply", it would have a link for
> "Reply as comment" and "Reply as review".

Two links instead of one? That makes it even harder for the user.

> > I also think the way that the links are piled up make it hard to find them.
> When I have been requested to review, I have a [Review] link, and a "Add a
> review or comment" very close to it.
>
> As you suggested recently, I'd like to have people choose which request
> their review satisfies using a dropdown.
>
> That would mean we could remove the [Review] links, which is what I plan
> to do.

I think that's great. If you want to land this, we need to fix this layout issue though.

> > Makes me think that, leaving aside the issue mentioned before, that link
> should at a very minimum be outside the table.
>
> I can do that, but I deliberately placed it as close to the revi...

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Albisetti wrote:
>>> I feel that going down the path of "Add a review or comment" has not been
>> good. Reviewing and commenting are very different things, and mixing them just
>> creates confusion.
>>
>> Thanks for your review.
>>
>> Can you explain what makes them different? To me, a review is just a
>> comment with a little bit of extra information. It forms part of a
>> discussion, which is why I want people to be able to perform a review in
>> reply to a comment.
>
> I've been involved in many MPs where I just want to comment, not weigh in on the decision.

I can think of four things you might put in a comment
- - response to the proposed merge
- - request for information
- - response to a comment
- - something off-topic

An response to the proposed merge is going to be a kind of review. It
may be incomplete, it may not be binding, but it's a review. You
comment because you want the issues you raise to be considered when
deciding whether to merge.

A request for information can be viewed as a review that the merge
proposal is inadequately specified.

A response to a comment will usually be a review in itself. Only the
proposer will regularly respond to requests for information. A response
to the proposed merge is a review, so a response to a response is itself
a review.

By definition, I don't think we need to consider off-topic comments or
replies to off-topic comments. Leaving them aside, comments are just
weaker reviews.

> Setting a status is a strong message, and a lot of people don't feel they have that kind of authority.

Right, so those people don't need to set a status. They are still
trying to influence the decision, so the distinction is only a matter of
degree.

> Commenting helps to bring down the barrier of entry.

>> I think part of the confusion is that the dropdown says "-Select-",
>> implying the user must choose. It should say "Just a comment", "Not a
>> review", or something like that.
>
> Yes, I think this is part of the problem. Collapsing this section if you click on "Add a comment", and optionally allow you to expand may be a good middle-ground, but it may also end of feeling like "design by committee",

It does seem a bit much to hide a small section, especially one that
people on the review team would like to have open.

>>> I think we should clearly separate them into different links, and just
>> distinguish the page it goes to by having the review section collapsed or
>> expanded by default. That way we still allow people to do both, but lead them
>> down two different paths.
>>
>> I still feel that drawing such a strong distinction, and then showing it
>> to be unnecessary by allowing people to do either from either page
>> doesn't seem like a good approach to me.
>
> Cramming both options in one link because we can feels very wrong.

It's not because we can, it's because they're strongly-related things.

>> I should mention that people who reply often don't know whether they're
>> providing a review or a comment until after they've finished typing it
>> up. They only know whether they can make a verdict until after they've
>> fully explored t...

This needs more thought, I feel we're going in the wrong direction.