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

Fabiano Fidêncio fabiano at fidencio.org
Fri Dec 5 18:04:43 PST 2014


On Fri, Dec 5, 2014 at 11:08 PM, Marc-André Lureau
<marcandre.lureau at gmail.com> wrote:
> On Fri, Dec 5, 2014 at 10:38 PM, Jonathon Jongsma <jjongsma at redhat.com> wrote:
>> For what it's worth, I basically agree with Christophe and Jeremy.
>> (Although I think that describing it as "mandatory code review" is
>> over-stating the case a bit -- there is nothing but peer pressure and
>> polite requests preventing contributors from pushing unreviewed code).
>
> Peer pressure is precisely what one should try to avoid. If you think
> your change does not require second look, because it is trivial, then
> why would you do it? Now you will have to bother others repeatedly for
> the most basic thing that they might not even care about. I would
> rather work differently as I explained before, and as we did until
> now.

I agree with Marc-André here.
Usually I send my patches to review because I feel that I can learn
more with people's review. Usually I explicit ask for a review (even
in projects I can push my changes directly) in commits I'm not sure
what I'm doing/if I'm doing it in the best way.
OTOH, there is a limit that we can tolerate. If one push was
considered controversial, come to the ML and complain about it. If it
happens repeatedly, the person has to be conscious enough (or told by
someone) to avoid pushing stuff without review.
Personally I do believe that review process helps newcomers to get
used to the code, teach them what to expect from a code and how to
review a code, so reviewing process is usually a win-win. OTOH, if the
sender thinks {s,}he doesn't have much to learn *and* that h{is,er}
contribution is simple enough, just go ahead and push it, it should be
up to the maintainer.

Also, Jonathon said about the process. Ah, I'd love if we can use
bugzilla for attaching and reviewing patches, seriously. But that is
just my personal opinion. If we keep using ML, I'd like to ask, at
least, to have a "pushed as ..." message for every pushed patch. To be
sure that that patch is already in and I don't need to waste time
reviewing it.

Just to finish, IMHO, this patch, specifically, was not a trivial one.

Best Regards.
-- 
Fabiano Fidêncio


More information about the Spice-devel mailing list