Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48949 )
Change subject: flashchips.c: Add support for XMC new spi flash types ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/flashrom/+/48949/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/48949/3//COMMIT_MSG@16 PS3, Line 16: TEST=test Was a test actually performed?
e.g.,
`TEST=flash image, then read it back then erased successfully.`
Also no need to indent the commit message. For an example of a commit message format see https://review.coreboot.org/c/flashrom/+/44073
https://review.coreboot.org/c/flashrom/+/48949/1/flashchips.h File flashchips.h:
https://review.coreboot.org/c/flashrom/+/48949/1/flashchips.h@814 PS1, Line 814:
two tabs needed to line up with the rest from the looks of it.
Click "Resolve" on comments that were fixed.
https://review.coreboot.org/c/flashrom/+/48949/1/flashchips.c File flashchips.c:
https://review.coreboot.org/c/flashrom/+/48949/1/flashchips.c@19004 PS1, Line 19004: TEST_OK_PREW
Were these all tested? if not then you want `TEST_UNTESTED`. […]
I feel like the `TEST=` line should indicate these were not actually tested from the sounds of things. So in the commit message `TEST=builds` and for each chip `.tested = TEST_UNTESTED`.
-
Edward O'Callaghan (Code Review)