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 -- M. Sc. Nico Huber Senior Consultant SINA Software Development and Verification Division Defence secunet Security Networks AG Phone: +49-201-5454-3635, Fax: +49-201-5454-1325 E-Mail: nico.hu...@secunet.com Mergenthalerallee 77, 65760 Eschborn, Deutschland www.secunet.com _____________________________________________________________________ secunet Security Networks AG Registered at: Kurfuerstenstraße 58, 45138 Essen, Germany Amtsgericht Essen HRB 13615 Management Board: Axel Deininger (CEO), Torsten Henn, Dr. Kai Martius, Thomas Pleines Chairman of Supervisory Board: Ralf Wintergerst ______________________________________________________________________
0xBD56B4A4138B3CE3.asc
Description: application/pgp-keys
signature.asc
Description: OpenPGP digital signature
_______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
