On 03/22/2014 06:18 AM, ron minnich wrote:
On Fri, Mar 21, 2014 at 6:52 PM, Alex G. <mr.nuke...@gmail.com> wrote:

The bad-ness or good-ness of motives is relative. Note that I'm not
using "bad" in the sense of "evil". Let's look at the six gatekeeper idea:
  * Easier for commercial entities to upstream code, therefore faster
progress for coreboot (good motive). (a)
  * Easier for commercial entities to upstream code, therefore we can get
lazy even if code quality drops (bad motive). (b)

That's not the intent. The way you are stating this has a lot of
built-in assumptions, and you're mixing some things up. That's our
fault; the old rule is, that if someone did not understand what you
said, it's your fault, not theirs. So, speaking as one of the guys who
reviewed the email, I'm sorry it was not clearer.

So, first, the intent of the six gatekeeper idea is, in part, to be
sure we're being very careful about what goes in. We've had some cases
over the past 2-3 years where some inexperienced guys pushed 'commit'
on code that broke a bunch of boards -- in ways that were not obvious.
We would like to avoid that. As we've learned the hard way a few
times, there are not that many people who have the perspective of long
experience and a view of all the boards, and know how trivial changes
can have far-reaching consequences.

Hi

Ron, would you be so kind and identify by commit hash some of these code merges by "inexperienced guys" from last 2-3 years that "broke a bunch of boards". I would like to see from gerrit history how these problems were ultimately dealt with and hopefully we could all learn from the mistakes that were made.

I could list some breakage, but the ones I can remember were all submitted by experienced long-term commercial contributors, and would not exactly match the criteria of your argument. If we look closer, probably every suggested gatekeeper is involved with merging such commits.

For an extreme sample of review process try follow this:

AMD CAR: Fix issue with gcc 4.8.x
http://review.coreboot.org/#/c/4205/

We have everything there -- a change in CAR init already approved in Chromium git without testing, test results for boards the changed code is never built for, test results for boards the change is not built for because of a Kconfig option not being set. We have developers from Chromium, SAGE and AMD involved in review. We have a couple commmunity developers involved. And all this to avoid breakage in some mostly obsolete K8 or fam10 platform we have in the tree.

Some changes to review process are certainly welcome. I just hope Stefan would have included separate section of the current problems and more details of the goals his proposal tries to address. Even some of the obvious ones are not answered. For example, are there actual plans to bring coreboot trees from coreboot.org and Chromium back in-sync?

Regards,
Kyösti


--
coreboot mailing list: coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot

Reply via email to