[Spice-devel] Proposal: review branches (was Re: [vdagent-win PATCH v6 2/5] Initial rewrite of image conversion code)

[Christophe de Dinechin]

> On 25 Jul 2017, at 19:37, Christophe Fergeau <cfergeau at redhat.com> wrote:
> 
> On Tue, Jul 25, 2017 at 02:26:36PM +0200, Christophe de Dinechin wrote:
>>>>> As long as contributor keep pinging or resending his series, this is already the case.
>>>> 
>>>> As Frediano said at the beginning of the series, “I’m tired of hearing this reply”.
>>> 
>>> And this is not an actionable answer... My perception is that there
>>> rarely are 'ping' on old series. Does this mean we are doing a good job
>>> at reviews?
>> 
>> I think that we (but not I) are doing an OK job at reviews, but we apparently drop
>> some reviews, e.g. because they were too complex, or did not represent the
>> priority of the time.
>> 
>> That being said, I observe that there are better ways to track WIP than pure mail.
>> Redmine, JIRA, pull requests, whatever. All well known solutions to the problems
>> we complain about.
>> 
>> As an aside, these tools typically solve many other problems too, like being able to
>> record things to be done *before* there is a patch for them, or CI, or priorities, etc.
>> Frankly, Bugzilla + Mail brings me back a good 15 years ago or something.
>> 
>> I don’t care much about which tool we use. I do mind that we have none.
> 
> The patch submitter's mind who sends ping when the series gets too old
> can be seen as such a tool, with the added benefit that they know if the
> series is still relevant, they can solve complex conflicts when
> rebasing, ... :)
> 
>>> (I doubt it or we would not have this conversation) Does
>>> this mean patch senders do not want to do that? Why? Does this mean it's
>>> done a lot, but to no avail? All I'm reading is "I'm not happy with how
>>> things work", with nothing specific.
>> 
>> It’s funny, because
>> a) I never said I was unhappy, and
>> b) I gave a very simple, very specific suggestion for action, which was to add a
>> URL to a branch with the name review/<author>/<topic> on freedesktop.org to the
>> cover letter or patch description.
>> 
>> So how you turn that into “I’m not happy with how things work with nothing specific”
>> is a bit beyond my understanding.
> 
> The quote (from you) on top of this part of my answer was
> « As Frediano said at the beginning of the series, “I’m tired of hearing
> this reply”. »
> I was specifically referring to that, which is non-specific, and which I
> interpret as unhappiness. I'll blame written media for any
> misinterpretation here :)

The good thing about written media is that we can go back for the full context, namely:

> 
>>>> I see several benefits to doing this:
>>>> 
>>>> 1. We always know exactly which component and branch is being patched
>>>> 
>> 
>> As long as contributor keep pinging or resending his series, this is already the case.
> 
> As Frediano said at the beginning of the series, “I’m tired of hearing this reply”.

I was simply quoting Frediano to indicate that he had already responded.

Ping is not a solution, it is a symptom of the problem. The problem being forgotten reviews.

Also, ping is a response for forgotten reviews, how does it help figuring out the
origin repository? Read again what I wrote, that’s clearly what I am talking about.

So I’m sorry, but it looks like you are giving a canned response, “ping”, that not
only ignores Frediano’s frustration with having to ping, but also ignores the problem
I’m bringing up as being also solved by git URLs, namely the difficulty to find
which repository a patch applies to. Hence my reusing the “I’m tired of” formulation :-)


>>> If yes, what is it? Patch reviews not being done in a timely
>>> manner? Patch series being forgotten? Patch series status hard to know
>>> by email? Something else? (note that you said "problem", not "problems"
>>> :)
>> 
>> Starting with the first in your list, but with the full knowledge of the state of the art
>> for tools solving this problem solves other problems that we discussed earlier at the
>> same time, and listing these problems as well.
> 
> Yes, but it's good to know what is the main issue people are having with
> the current workflow, so that we make sure this really is fixed by any change
> in tools and processes. The other features would then just be additional
> niceties.

I agree.

Now, any objection to

1. Recommending that we use git URLs in patches?
2. Having a shared location for branches under review?


Thanks,
Christophe

> 
> Christophe
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel