[Spice-devel] spice refactoring: workflow suggestion

[Victor Toso]

Hi,

On Mon, Nov 02, 2015 at 06:34:39AM -0500, Frediano Ziglio wrote:
>
> >
> > Hi,
> >
> > As in a mailing list it is a bit hard to track of the status of each
> > PATCH (acked/reviewed/pushed) and we still have tons of batch of
> > patches to apply/test/review I would like to suggest the following:
> >
> > 1-) When a batch is pushed, send an email in-reply-to 00/00 mail saying
> > that patches were pushed.
> > -- This really helps when one is behind following the changes
> >
>
> This can be done.
> On the same subject I thing the spreadsheet should be publicly available
> (read only). It mainly contains list of open source patches, progress,
> some report.
>

Great. I don't think sharing the spreadsheet is a problem.

> > 2-) Push series as a whole when acked instead of pushing each patch
> > when acked. In case a few patches of a series are taking time but a
> > few have been acked, maybe pushing those acked but let us know with
> > an email
> > -- Reviewing a patch to later on see that it was pushed already means
> > time lost. I believe an email is quicker then matching which patch awas
> > pushed or not.
> >
>
> These series are quite different from normal ones as the patches inside
> are quite different one from another, they don't share the same aim.
> It never happened that all patches was accepted at once.
> I agree a reply saying "pushed" could work.
>

Indeed. Having a way to track which patch was pushed or not seems good
enough to me.

(PS: I spent some time taking a look in our patchwork [0] and I think it
is just a matter of using the right tools [1] to make this interface all
we need to track the patches... but I'm not sure yet.

[0] https://patchwork.freedesktop.org/project/Spice/list/
[1] such as git-pw
http://patchwork-freedesktop.readthedocs.org/en/latest/manual.html#git-pw

> > 3-) Might sound silly but as this involves lots of patchs in a short
> > period, it might be interesting to have acked-by, tested-by, reviewed-by
> > ? At least for the refactoring...
> >
>
> I add the Acked-by. Nobody seems to review but not ack to but if somebody
> want to distinguish the two cases is free to tell review instead of ack.
> Do you think we should add a reviewed-by line for each people that commented
> the patch?

Not really, only for those who reply with reviewed-by I guess.

My point is just adding informationg to better track changes. Many
changes require more information to check who was involved.

This could be a bit annoying/disnecessary but I thought it would be
worth mentioning it here.

> I don't know if is worth to add the tested-by. I do some small test before
> committing but is really dirty and fast. I mean, style/move patches you can
> just test that is mainly compiling, working but fix for instance would
> require reproduction and I don't think this is done strictly for all fix
> (we must take also in consideration there are really few fixes compared
> to other changes).
> Just to not scare people on testing we run quite often some automated test
> and I/we ran different tests for leaks and memory errors but more
> periodically than for single patches.
>

Yeah, tested-by only seems interesting to have if one does some extent
tests with each patches. As it is automated it is probably not
necessary.

> > Any thoughts?
> > Regards,
> >   Victor Toso
>
> I really agree something is wrong in the process.
>
> I have the sensation that the acknowledge process is a bit weird and
> different than usual. One reason is that author is not changing the
> patches that are changed by somebody else. This makes the editor a
> co-author removing him the theoretical possibility to ack the patch.

Not sure how one can handle this in a better way.
  - toso