[Spice-devel] [PATCH spice-protocol] build-sys: simplify autogen

[Marc-André Lureau]

On Fri, Dec 5, 2014 at 5:43 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> On Fri, Dec 05, 2014 at 04:52:49PM +0100, Marc-André Lureau wrote:
>> "Use autoreconf, allow out of tree autogen.sh run."
>
> Nowhere it says that you just got rid of the existing autogen.sh and
> replaced it with something else, you only say 'simplify'

It is a summary, "use autoreconf" mean we don't have much to do in
autogen.sh, this should be obvious to you too.

Looking at the change or the result is still the most important thing.
A commit message wouldn't be a summary otherwise, you have to look at
the diff to know the details, as always.

>> That's a big factor, yes.
>
> What is so bad with having a commit delayed for a few hours while it's
> waiting for reviews?

It is mainly the difference between asking someone to take an action
or not. And asking someone take time often longer than necessary.

>> Also, you can see that other projects have troubles with that, ex
>> http://wiki.qemu.org/Contribute/TrivialPatches: they are moving the
>> problem to me, but perhaps it helps.
>
> QEMU is a much bigger project though. I don't think there is much point
> looking at other projects, some are doing mandatory reviews, some are
> doing free for all commits, some are using mailing lists, others are
> using tools for patch tracking, ... And some are doing exactly what we
> do currently, with everyone agreeing to send all their patches for
> review before pushing.

And Spice is no such a big project with so many contributors, your
argument can be reverted.

>> > With pre-commit review, you ensure that at least one person read the
>> > patch. With post-commit review, you have no such guarantee.
>>
>> With current workflow, you have no guaratee that unreviewed patch go
>> there by mistake or maliciously. We would need a tool for that.
>> For me this is the job of maintainer to quickly review each commit
>> before release.
>
> Not sure what you are getting at, 'quickly review' here could mean
> glance at the shortlog given the context. And this definitely does not
> scale, and you don't want to discover issues at release time. We also
> have spice-commits for this scenario.

I mean that we need to do post-commit review nonetheless unless you do
signed commit with a tool and ACL. I really don't think our project
deserve that treatment.

>> Btw, do you apply your own rules in your own projects?
>
> I don't really have a project of mine in mind where a second person is
> actively involved, which is a prerequisite to set up such a system.
> I'd be happy to switch to that model if I had the opportunity. If
> someone was to take over libgovirt maintainance, I'd be happy to send
> patches before committing.

libgovirt is used by virt-viewer, and it means the Spice and
virt-tools team are affected by your changes. You can always ask for
help, but you trust enough your own work to do it alon and provide it
to those projects that will rely on it. Those projects trust you to do
the right thing by yourslef. Similarly, I would like you trust
contributors to do the right thing for Spice, without mandatory code
review. There has been no big issues so far, only you complaining for
no valid technical reasons. Let's stick to that before enforcing
rules.

-- 
Marc-André Lureau