On Wed, Nov 06, 2019 at 12:39:59PM +0100, Nico Huber wrote:
Some of the mega patches are copies of a predecessor chip (with the minimum amount of changes to integrate it in the build), that are then modified to fit the new chip.
Ack. I think that is a problem. If this procedure is intended, I think we should update our guidelines to reflect that.
I guess first we should get on the same page with regard to strategy. There's a bit of flip-flopping between extremes (code duplication vs. silently breaking stuff).
Once we're there, we should check if documentation can be improved (very likely yes).
That was considered an acceptable strategy for a long time (after we broke up 82801xx because fixing one variant broke another). We can certainly revisit that, but to me that change appears to have happened gradually and without much notice.
That's news to me. I didn't witness this break up. And it looks like
That break up was 2010, which I think is before your coreboot time. See commit 138be8315b63b0c8955159580d085e7621882b95
Was there any public discussion that I could look up?
That's not how the project worked back then.
What I'd like to figure out most is how our Gerrit guidelines are supposed to be interpreted for a review of such a patch. I see no exception for not reviewing such patches and have no idea how one could review one. Also what would be the responsibilities of a reviewer after review?
I suppose the criterion for such a commit is "is the diff to the original chipset code reasonable" (that is, changes names and stuff so it builds in parallel, but nothing else). Does that make sense?
Patrick