Launchpad reviews (was Re: Patch Pilot report)

Tue Nov 24 06:10:49 GMT 2009

Yes.

> I don't think the terminology is ideal, and I have filed a bug to that
> effect, and I think also one asking that you should document what they
> do mean.  But more importantly the emergent behaviour does not align
> with the way you expect it to work: for example
> <https://code.edge.launchpad.net/~vila/bzr/releasing-clarified/+merge/10854>
> is shown in "reviews I can do" but in fact there is nothing useful for
> me to do there.

I'm not sure why there is an outstanding review request for the team.
Normally, Ian's review would automatically be selected as the bzr-core
review.  Perhaps two reviews were requested of bzr-core?

Given that there *is* an outstanding review request for a team that
you're on, I hope you'd agree that it should be presented as something
you can review.

> Updating a bug does naturally lead you into changing its state and
> priority, which determine what happens to it next.

I don't follow.  Changing the state and priority have an impact on
humans, who determine what to do next based on the project's
conventions.  The "what's next" part isn't Launchpad per se.

>>>  * I will do the changes and land it
>> I think that would be best expressed by creating your own branch and
>> creating a new merge proposal that supersedes this one.  The 'supersede'
>> part isn't something we currently support.
>>
>>>  * I want someone to do the changes and land it, but not me
>>>  * I want the submitter to do the changes and land it (not sure what
>>> status this is)
>> "needs-fixing"
> 
> These three cases have quite different implications for the next
> action, and I think the fact they're conflated causes some churn.

Which three cases are conflated?

> 
>>>  * I want the submitter to do the changes and resubmit it (I guess
>>> this is 'needs fixing'?)
>> It is "resubmit".
> 
> Perhaps this changed, but 'resubmit' used to have the special
> behaviour of creating a new mp, not asking someone to resubmit.

The "resubmit" vote has never had that effect.  There used to be a
"resubmit" entry in the status selector, but that has been turned into
the "resubmit proposal" action.

> I've been involved with this for something over four years

Presumably, you're including Bundle Buggy, which also used "resubmit"
for the purpose?

> and I've
> done probably over a hundred reviews in the new system.  I'm not being
> intentionally bloody minded.  So if I'm not sure what the statuses
> mean, and apparently most other bzr core team members don't either, I
> think there is a real problem.

I think you have a lot of opportunities to find out about the system, so
if you don't understand it now, you haven't taken advantage of them.

>> The code review system is not intended to enforce a particular workflow;
>> it's meant to provide enough flexibility to enable teams to use it as
>> they see fit.
> 
> Let's be clear, I'm not asking for you to enforce a particular
> workflow or to remove any generality.  I don't dispute that it's
> technically possible to use it.  It's also possible to do reviews on
> the mailing list or by attaching them to bugs.  The whole point of the
> review system is to help with workflow, so user feedback that it does
> not support their workflow well is pretty important.

But the significance of that feedback also depends on the degree of
understanding demonstrated by the user.  The critique of a user who does
not thoroughly understand the system may simply indicate a need for
documentation or education.

> The specific changes I'd like to see are:

(These are a lot more palatable than the "suggest a next action out of
these 10 or so" thing that I was responding to)

>  * document the headings in the overview (either in help or inline)
>  * document the votes and statuses
>  * address the problem of people creating reviews as themselves when
> they want to review on behalf of a team

I'm not sure what you mean.  You mean that when there is a review
request for bzr-core, and you perform a review, that review request
isn't converted into a review request for Martin Pool?  Or do you mean
that when you perform such a review, you want it to be attributed to the
team?  As well as yourself?

>  * add a field to the review form to set the new status of the mp

I don't think we want something that general.  We wouldn't want people
voting "disapprove" and selecting "approve" as the status, for example.

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

iEYEARECAAYFAksLeOUACgkQ0F+nu1YWqI3gAwCdGe/BWVqvIxz0RNI+R/Eje4cV
9xIAn3oh9LCrFeE35xqcaFE8oVc5HsSk
=oDwi
-----END PGP SIGNATURE-----