Change in coreboot[master]: mb/*/Kconfig: Remove redundant 'MAINBOARD_VENDOR' type define
HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45121 )
Change subject: mb/*/Kconfig: Remove redundant 'MAINBOARD_VENDOR' type define ......................................................................
mb/*/Kconfig: Remove redundant 'MAINBOARD_VENDOR' type define
MAINBOARD_VENDOR is already declared as a string.
Change-Id: If6b565c538d751e06207d39c5da3f12ae800f385 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/51nb/Kconfig M src/mainboard/dell/Kconfig M src/mainboard/libretrend/Kconfig M src/mainboard/ocp/Kconfig M src/mainboard/prodrive/Kconfig 5 files changed, 0 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/45121/1
diff --git a/src/mainboard/51nb/Kconfig b/src/mainboard/51nb/Kconfig index 5124fdb..d0efbc6 100644 --- a/src/mainboard/51nb/Kconfig +++ b/src/mainboard/51nb/Kconfig @@ -12,7 +12,6 @@ source "src/mainboard/51nb/*/Kconfig"
config MAINBOARD_VENDOR - string "Mainboard Vendor" default "51NB"
endif # VENDOR_51NB diff --git a/src/mainboard/dell/Kconfig b/src/mainboard/dell/Kconfig index 298c62b..d95850d 100644 --- a/src/mainboard/dell/Kconfig +++ b/src/mainboard/dell/Kconfig @@ -10,7 +10,6 @@ source "src/mainboard/dell/*/Kconfig"
config MAINBOARD_VENDOR - string default "Dell Inc."
endif # VENDOR_DELL diff --git a/src/mainboard/libretrend/Kconfig b/src/mainboard/libretrend/Kconfig index 7e1eacd..0612ec7 100644 --- a/src/mainboard/libretrend/Kconfig +++ b/src/mainboard/libretrend/Kconfig @@ -10,7 +10,6 @@ source "src/mainboard/libretrend/*/Kconfig"
config MAINBOARD_VENDOR - string "Mainboard Vendor" default "Libretrend"
endif # VENDOR_LIBRETREND diff --git a/src/mainboard/ocp/Kconfig b/src/mainboard/ocp/Kconfig index b748129..9269bfa 100644 --- a/src/mainboard/ocp/Kconfig +++ b/src/mainboard/ocp/Kconfig @@ -10,7 +10,6 @@ source "src/mainboard/ocp/*/Kconfig"
config MAINBOARD_VENDOR - string default "Open Compute Project"
endif # VENDOR_OCP diff --git a/src/mainboard/prodrive/Kconfig b/src/mainboard/prodrive/Kconfig index 0847425..62524d0 100644 --- a/src/mainboard/prodrive/Kconfig +++ b/src/mainboard/prodrive/Kconfig @@ -10,7 +10,6 @@ source "src/mainboard/prodrive/*/Kconfig"
config MAINBOARD_VENDOR - string default "Prodrive"
endif # VENDOR_PRODRIVE
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45121
to look at the new patch set (#2).
Change subject: mb/*/Kconfig: Remove redundant 'MAINBOARD_VENDOR' type define ......................................................................
mb/*/Kconfig: Remove redundant 'MAINBOARD_VENDOR' type define
MAINBOARD_VENDOR is already declared in src/mainboard/Kconfig file.
Change-Id: If6b565c538d751e06207d39c5da3f12ae800f385 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/51nb/Kconfig M src/mainboard/Kconfig M src/mainboard/dell/Kconfig M src/mainboard/libretrend/Kconfig M src/mainboard/ocp/Kconfig M src/mainboard/prodrive/Kconfig 6 files changed, 2 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/45121/2
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45121
to look at the new patch set (#3).
Change subject: mb/*/Kconfig: Remove redundant 'MAINBOARD_VENDOR' type define ......................................................................
mb/*/Kconfig: Remove redundant 'MAINBOARD_VENDOR' type define
MAINBOARD_VENDOR is already declared in src/mainboard/Kconfig file.
Change-Id: If6b565c538d751e06207d39c5da3f12ae800f385 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/51nb/Kconfig M src/mainboard/Kconfig M src/mainboard/dell/Kconfig M src/mainboard/libretrend/Kconfig M src/mainboard/ocp/Kconfig M src/mainboard/prodrive/Kconfig M util/autoport/main.go 7 files changed, 2 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/45121/3
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45121 )
Change subject: mb/*/Kconfig: Remove redundant 'MAINBOARD_VENDOR' type define ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45121/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45121/4//COMMIT_MSG@9 PS4, Line 9: MAINBOARD_VENDOR is already declared in src/mainboard/Kconfig file. If we want to make it a rule (so far I considered it personal taste), we should document that at least. The "Notes" below `### default` in `Documentation/getting_started/kconfig.md` seem a good place.
https://review.coreboot.org/c/coreboot/+/45121/4/src/mainboard/Kconfig File src/mainboard/Kconfig:
https://review.coreboot.org/c/coreboot/+/45121/4/src/mainboard/Kconfig@14 PS4, Line 14: source "src/mainboard/*/Kconfig" Why move it? I guess it doesn't break because there is no default given here, but usual order in Kconfig is: 1. define the default (because the first default takes precedence), 2. declare the Kconfig symbol where it is used / where it should show up in case it has a prompt.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45121 )
Change subject: mb/*/Kconfig: Remove redundant 'MAINBOARD_VENDOR' type define ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45121/4/src/mainboard/Kconfig File src/mainboard/Kconfig:
https://review.coreboot.org/c/coreboot/+/45121/4/src/mainboard/Kconfig@14 PS4, Line 14: source "src/mainboard/*/Kconfig"
Why move it? I guess it doesn't break because there is no default given […]
when you run 'make menuconfig' and search for MAINBOARD_VENDOR , it will see it defined at "src/mainboard/51nb/Kconfig" which is odd.
with current patch, we will see MAINBOARD_VENDOR defined as string at "src/mainboard/Kconfig"
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45121 )
Change subject: mb/*/Kconfig: Remove redundant 'MAINBOARD_VENDOR' type define ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45121/4/src/mainboard/Kconfig File src/mainboard/Kconfig:
https://review.coreboot.org/c/coreboot/+/45121/4/src/mainboard/Kconfig@14 PS4, Line 14: source "src/mainboard/*/Kconfig"
when you run 'make menuconfig' and search for MAINBOARD_VENDOR , it will see it defined at "src/main […]
Interesting. Also something to document. e.g. If possible, the declaration should happen before all default entries to make it visible in kconfig tools like `menuconfig`.
HAOUAS Elyes has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/45121 )
Change subject: mb/*/Kconfig: Remove redundant 'MAINBOARD_VENDOR' type define ......................................................................
Abandoned
-
HAOUAS Elyes (Code Review) -
Nico Huber (Code Review)