Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36087 )
Change subject: soc/intel/tigerlake: Do initial SoC commit ......................................................................
Patch Set 8:
Patch Set 8:
Oooops, I think people are talking past each other here.
Subrata, nobody suggested that we shouldn't create a tigerlake/ directory, we should!
Arthur, nobody suggested to review this patch. I assume Subrata deliberately ignores our development guidelines and wants this rubber-stamped.
Our problem is not with the introduction of 8k LOC for Tigerlake. Sure, more common code would be nice, but we understand that it's not the best time. The problem is that you want to skip review. Which multiplies future maintenance effort, just to save somebody some days to write proper commits. That's very harmful for the project as a whole. Without a reasonable commit history (from which one can tell if a piece of code was reviewed for a particular piece of silicon) maintenance cost increase so heavily that it delays future work like more common code and eventually also the next SoC generation. It also increases the regression rate, it wastes resources on bug hunting, ... A broken Git history is a technical debt that can only be repaid by starting from scratch. I doubt that is the plan here.
So, instead of adding 8k LOC at once and unreviewed, we want you to add the code topic by topic, and let it be reviewed commit by commit. That shouldn't be hard. Just take the current state you have achieved, throw everything away that isn't validated yet, and put what is left into smaller, reviewable commits. Please!
its reasonable to add code block by block but end of 50+ CL then we will see ultimately we have same 8K LOC in TGL as existed today in ICL. rather can we have areas identified as arthur has mentioned should be good to fix in ICL code itself then copy into TGL ?
-
Subrata Banik (Code Review)