Re: September 2015 Commitfest

On Sat, Oct 31, 2015 at 03:43:13PM +0000, Nathan Wagner wrote:
> There was then some further discussion on the interface, and what to do
> with startup files, and nothing was really decided, and then the thread
> petered out. This potential reviewer is then left with the conclusion
> that this patch really can't be reviewed, and it's not sure if it's even
> wanted as is anyway. So I move on to something else. There was a bunch
> of discussion by a bunch of committers, and no-one updated the status of
> the patch in the commitfest, and there's no clear guidelines on what the
> status should be in this case.

There are no clear guidelines, agreed.

> If I were needing to decide, I would say that the patch should either be
> marked as "Waiting on Author" on the grounds that the patch doesn't
> currently apply, or "Returned with feedback" on the grounds that there
> was unaddressed feedback on the details of the patch, and it was noted
> as a "proof of concept" when submitted anyway. But I'm unwilling to
> just change it to that without more clear definitions of the meaning of
> each status. A link to definitions and when the status should be
> changed would help.

By default, the author is responsible for driving the thread to a conclusion.
If a patch still has "Needs review" status after an inconclusive discussion
petered out, I would set it to "Waiting on Author".

Changing directly from "Needs review" to "Returned with feedback" is unusual;
folks do so when a review calls for extensive edits approaching a full
rewrite. More often, a patch first spends some time in "Waiting on Author".

> What is "ready for committer" anyway? It's clearly not "a committer
> will apply the patch", since things sit in that status without being
> committed.

"Ready for Committer" is the reviewer saying, "Based on my review, I would
have committed this if I had been the patch's committer." You can further
qualify that statement in your review message, e.g. "I'm marking this RfC, but
I'm not sure about the spinlock usage in function FooBar()."

> If I think the patch is good and should be applied, do I
> mark it as ready for committer, and then later a committer will also
> review the patch? If so, is doing anything other than the sanity
> checks, and possibly offering an opinion, on the patch even useful?

Yes and yes; see Tom's reply for details.

Thanks,
nm