Am 05.11.19 um 20:23 schrieb Patrick Georgi:
On Tue, Nov 05, 2019 at 06:37:02PM +0100, Nico Huber wrote:
I mean "rubber-stamping of *huge* commits". That huge that it is obvious that no review happened, e.g. 1k+ LOC copy-pasta. Also, the guidelines say "This means you shouldn't +2 a patch just because you trust the author of a patch [...]" I didn't want to quote the whole paragraph, maybe I should have ;)
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.
The idea was precisely to ensure that changes between the generations are visible in the history.
That's nice for historians, but what about maintenance? There is nothing in the Git history if a piece of code was intended/validated/reviewed for a given chip (unless it was adapted which is the less common case).
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 afterwards, new chipsets were added with already patched code bases. The first unpatched copy I could find (didn't look very long) is soc/intel/braswell/.
Was there any public discussion that I could look up?
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?
Nico