Hi Martin,
Martin Roth via coreboot wrote:
..
It's not a complete solution, but hopefully it's seen as a step in the right direction.
Thanks a lot for this.
I think that this really helps with visibility while guiding people at the same time. Great idea!
I have some comments: (apologies for not using gerrit)
* Managing expectations
"Run the command 'git checkout $(CONFIG_MAINBOARD_BRANCH)' to build this board."
The git command doesn't build anything, so maybe ".. to switch branch." or ".. first and then re-run Kconfig to set up your board." or somesuch?
* Most of src/Kconfig now seems excluded for branch boards, maybe:
if MAINBOARD_SUPPORTED_ON_BRANCH source "src/mainboard/Kconfig" source "site-local/Kconfig" else # everything endif
rather than multiple if-endif statements is less confusing? (Yes, the duplicated source lines aren't great, but maybe okay for two lines?)
* Branch names
Newer git branch names look like "4.11_branch" so src/Kconfig and Kconfig.branch saying "(supported on 4.11 branch)" could really trap people, especially with "4.11" being a valid git tag.
But including the branch name also in the bool description is already redundant and could conflict with select _ON_BRANCH_*.
I really like the comment idea in src/Kconfig to connect _ON_BRANCH_* configs with the branch name only in a single place!
Do you see any way to avoid all the "(supported on * branch)" instances, while still making the information available in the vendor menu? Hmm.
* Having both Kconfig.name + Kconfig.branch
The idea is for SHOW_BRANCH_SUPPORTED_PLATFORMS to gate board visibility in Kconfig for "forward-movers", right? (Maybe name it _SUPPORTED_BOARDS btw.?)
IIUC the "if SHOW_BRANCH_SUPPORTED_PLATFORMS" logic would need to sometimes go into Kconfig.name (all boards on branches) and other times into Kconfig.branch (only some boards on branches), that seems a bit fiddly.
Might it be better to only use Kconfig.name and not introduce a new Kconfig.branch?
And maybe it would even be better to introduce this SHOW_BRANCH_SUPPORTED_PLATFORMS in a commit of its own?
Thanks again and kind regards
//Peter