Attention is currently required from: Nico Huber, Thomas Heijligen. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58468 )
Change subject: Makefile, ich_descriptor_tool/Makefile: unify behavior ......................................................................
Patch Set 2: Code-Review+1
(8 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/58468/comment/9634aead_9e046e9f PS2, Line 7: descriptor nit: missing plural `s` in `descriptors`
https://review.coreboot.org/c/flashrom/+/58468/comment/24c8b972_ab7bef3a PS2, Line 9: remove use gerund form: `removing`
https://review.coreboot.org/c/flashrom/+/58468/comment/43774a7f_e639a7bc PS2, Line 12: descriptor nit: missing plural `s` in `descriptors`
https://review.coreboot.org/c/flashrom/+/58468/comment/e39a513a_43f51a34 PS2, Line 13: Makefie typo: missing `l` in `Makefile`
https://review.coreboot.org/c/flashrom/+/58468/comment/ae193be4_87ba5757 PS2, Line 16: set CFLAGS and EXEC_SUFFIX for TARGET_OS not for the HOST_OS Two nits:
- Drop `the` in `for the HOST_OS` for consistency with `for TARGET_OS` - Add a comma after `TARGET_OS`
set CFLAGS and EXEC_SUFFIX for TARGET_OS, not for HOST_OS
https://review.coreboot.org/c/flashrom/+/58468/comment/48f95a1c_1d2cd81d PS2, Line 19: descriptor nit: missing plural `s` in `descriptors`
https://review.coreboot.org/c/flashrom/+/58468/comment/04447544_2787905c PS2, Line 18: Don't pass the EXEC_SUFFIX from the main Makefile to : ich_descriptor_tool/Makefile. I see that `TARGET_OS` is no longer passed either. I imagine it wasn't mentioned in the commit message because `util/ich_descriptors_tool/Makefile` overrides it anyway.
Still, I'd mention `TARGET_OS` in the commit message.
File util/ich_descriptors_tool/Makefile:
https://review.coreboot.org/c/flashrom/+/58468/comment/d7472dfe_f7cd5986 PS2, Line 42: */ nit: remove?
-
Angel Pons (Code Review)