On 03/22/2014 06:18 AM, ron minnich wrote:
On Fri, Mar 21, 2014 at 6:52 PM, Alex G. mr.nuke.me@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
* Kyösti Mälkki kyosti.malkki@gmail.com [140322 18:44]:
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?
Kyösti,
thanks a lot for your input. I will try to provide examples in the future.
To answer your question, yes, there are very concrete plans of syncing up the coreboot.org and Chromium coreboot trees happening right now. This is where part of this is coming from, and I am working on both sides of the story to resolve this.
I am also working on getting Google's internal upstreaming process for coreboot straightened out and making the resources available so that upstreaming happens more timely in the future, and rebasing the chromium tree more against a current upstream version more often. To throw some unconfirmed numbers in here, the current chromium tree is based off of a late 2012 upstream tree (though we have pulled a lot of patches in both directions since then). The ideal would be to move the chromium tree to a new coreboot upstream in an interval of something like 3-6 months.
Stefan
Dear coreboot folks,
Am Samstag, den 22.03.2014, 19:44 +0200 schrieb Kyösti Mälkki:
On 03/22/2014 06:18 AM, ron minnich wrote:
On Fri, Mar 21, 2014 at 6:52 PM, Alex G. mr.nuke.me@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 am interested in this too, as I heard this argument very often, but according to my memory the claim is not true.
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.
[…]
Thanks,
Paul
Paul, please ignore my top-posting. Could you please not hijack threads as long and tedious as this one? Feel free to quote the comments you are referring to, but make sure to scrub the in-reply-to field. It's getting tedious to read this.
Oh, and probably a lot of people have muted this thread already. You won't reach your intended audience.
On Saturday, March 29, 2014 12:34:19 AM Paul Menzel wrote:
Am Samstag, den 22.03.2014, 19:44 +0200 schrieb Kyösti Mälkki:
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 am interested in this too, as I heard this argument very often, but according to my memory the claim is not true.
Everybody makes commits which break boards. Sure, you'll be able to come up with a few hashes that caused issues and were merged by less-experienced people. If we look hard enough, we'll find the same is true of guru and god devels as well. Things break. It's a natural part of the process.
(Just had a deja-vu when typing this. Did I say this before?)
Clumsifying the process because things occasionally break is not going to unbreak those things, nor is it going to stop breakages in the future. We've seen this with TSA, terrorist threats, and even coreboot. No matter how much better the new process is compared to the previous, things still break.
Alex
-
Kyösti Mälkki -
mrnuke -
Paul Menzel -
Stefan Reinauer