No status for "basically ok, some questions"

[Jonathan Lange]

When doing code reviews, I often come across basically ok patches with some questions I don't understand. When I'm doing a review through the web, the review form insists I choose a review response type:

    * 'Approve' is inappropriate, since I can't approve of it until I understand it.
    * 'Needs Fixing' doesn't really match either, since the patch might well be fine.
    * 'Abstain' is wrong, since I am going to vote, just not yet.
    * 'Disapprove' is definitely out, since it looks basically good.
    * 'Resubmit' isn't very clear. I don't necessarily want a new patch, just some answers.

[Martin Albisetti (beuno)]

I've felt that way a few times as well. I end up just commenting, but I agree that a status that says "I looked at it, but please tell me more" would feel better.
"Needs Info"?

[Aaron Bentley (abentley)]

To me, this is a classic example of a comment. You're not ready to review until you've got the answers.

[Jonathan Lange (jml)]

On Tue, Feb 10, 2009 at 1:17 AM, Aaron Bentley <email address hidden> wrote:
> To me, this is a classic example of a comment. You're not ready to
> review until you've got the answers.
>

Interesting.

A comment doesn't address the workflow situation: I want to indicate
that I'm blocked until I get answers, and that the patch submitter
needs to do something.

[Tim Penhey (thumper)]

On Tue, 10 Feb 2009 11:10:21 Jonathan Lange wrote:
> On Tue, Feb 10, 2009 at 1:17 AM, Aaron Bentley <email address hidden> wrote:
> > To me, this is a classic example of a comment. You're not ready to
> > review until you've got the answers.
>
> Interesting.
>
> A comment doesn't address the workflow situation: I want to indicate
> that I'm blocked until I get answers, and that the patch submitter
> needs to do something.

Well if you're blocked is that "needs fixing"?

Or perhaps you're after a "needs feedback", or "needs response"?

Tim

[Jonathan Lange (jml)]

On Tue, Feb 10, 2009 at 12:58 PM, Tim Penhey <email address hidden> wrote:
> On Tue, 10 Feb 2009 11:10:21 Jonathan Lange wrote:
>> On Tue, Feb 10, 2009 at 1:17 AM, Aaron Bentley <email address hidden> wrote:
>> > To me, this is a classic example of a comment. You're not ready to
>> > review until you've got the answers.
>>
>> Interesting.
>>
>> A comment doesn't address the workflow situation: I want to indicate
>> that I'm blocked until I get answers, and that the patch submitter
>> needs to do something.
>
> Well if you're blocked is that "needs fixing"?
>

That's close, but as stated earlier, the patch doesn't necessarily
need fixing, so it's not true and sounds a little too harsh.

> Or perhaps you're after a "needs feedback", or "needs response"?
>

That's right -- "needs information" is what Martin A suggested.

jml

[Martin Pool (mbp)]

bb has 'tweak' which might not quite be the same, but sounds a bit
different to 'needs fixing'.

--
Martin <http://launchpad.net/~mbp/>

[Martin Pool (mbp)]

See bug 373038 asking for 'tweak'