Code review comment for lp:~abentley/launchpad/mp-tweakage

-----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 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 agree. I don't want to have different links for commenting and
reviewing. But if you insist on having them, then we should provide
them everywhere it makes sense, and it definitely makes sense to review
in reply to a comment.

>>> 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.

Do you mean that removing the [Review] links would not fix this layout
issue? What else could we do to fix it?

>>> 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.
>
> Could you try it out?

Sure.

>>> 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 cover letter, resubmit is placed next to the
>> information that it affects, so it's not random even if that's how it feels.
>
> I think it's not so obvious to the user, and thus the global actions portlet is more appropriate.

This sort of thing is specifically forbidden in the 2.0 layout, right?

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

iEYEARECAAYFAkp4W+AACgkQ0F+nu1YWqI1BLQCfVjMXjVy1iChd8gT9AlyOl8L9
oC8An3wLVhjFTy/MTZNIdBHtQdpxT7zl
=cCef
-----END PGP SIGNATURE-----