Attention is currently required from: Jakub Czapiga, Paul Fagerburg, Jan Dabros. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52937 )
Change subject: tests: Enable config override for tests ......................................................................
Patch Set 1:
(5 comments)
Patchset:
PS1: Whoops, I actually wrote the same thing up a few weeks ago but then got distracted and didn't get around to uploading it. Oh well. Good to see we came up with almost the same implementation, at least.
File tests/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/52937/comment/38b1af23_46e1afd4 PS1, Line 88: $(eval $(1)-config-file := $(obj)/$(1)/config.h) I'm confused why this needs to be an eval? I think you can just do
$(1)-config-file := ...
here, but then you should use
$$($(1)-config-file)
to refer to it later.
https://review.coreboot.org/c/coreboot/+/52937/comment/39e825fc_1bc10618 PS1, Line 89: $($(1)-config-file): $(TEST_KCONFIG_AUTOHEADER) nit: Doesn't really need to be a separate line on its own.
https://review.coreboot.org/c/coreboot/+/52937/comment/36651acd_ec6559f5 PS1, Line 97: printf '#ifdef %s\n' "$$$$key" >> $$@; \ nit: don't really need the #ifdef wrapper around the #undef
https://review.coreboot.org/c/coreboot/+/52937/comment/ea60253d_f46d9c7c PS1, Line 107: $(TEST_KCONFIG_AUTOHEADER) Just replace this with $(1)-config-file, then you don't need the extra line above
-
Julius Werner (Code Review)