'Pending' patches state

[Michael Stefaniuc]

Alexandre,

On 03/28/2012 10:17 AM, Alexandre Julliard wrote:
> Dmitry Timoshkov <dmitry at baikal.ru> writes:
> 
>> It's very confusing, and absolutely not clear what is required from the
>> patch submitter, especially since *there is no any feedback on the patch*.
>> 'Rejected' at least requies some sort of feedback, while 'Pending' doesn't.
>> To me 'Pending' looks like a silent case of 'Reject', but without any
>> obligation to explain why.
>>
>> I find myself on somewhat shaky ground when I see a bunch of my patches
>> in the pending state, especially if they already contain the tests and
>> main developer in the area I'm changing is Alexandre :).
>>
>> Is it possible to get at least some feedback for pending patches? Pretty
>> please?
> 
> The pending state is feedback. It means that the patch is not clearly
yes, but the worst possible feedback.

New people assume you or the area maintainer need to still make up their
mind on the patch but that's not the case, it is a done deal.

> correct, but that it's complicated to articulate exactly why. Like it
> says, you should try to make it more convincing, either by simplifying
> the patch, or writing a test case.
Iff one knows you and knows to read in between the lines then the above
can be reworded as:
  "Current implementation rejected; idea might have some merit."

That's what "Pending" means most of the times.

Now for the other meanings of "Pending":
- "Waiting for feedback from the main developer in that area."
  Don't remember to have ever seen that. Those patches stay normally in
"New" as you don't look at them before the area developer ACKs.

- "preferably in the form of a test case"
  That should be "Needs tests".

- With the two (minor) other meanings of "Pending" handled by other
patch states we can rename "Pending" to "Convince me" or "Needs work".
That would make it more obvious what is meant.

> For instance your patch 84692 says that "tests confirm that", but you
> don't say which tests, and there are no new tests or fixed todos in the
> patch, so it looks suspicious. Yes, I could dig out the tests myself and
> investigate it in detail, but when it gets to that point I usually just
> move on to the next patch, hence "pending".
IMHO that should have been "Needs tests"; that would have forced Dmitry
to point you to the tests.

bye
	michael