Hi Patrick,
On 26.01.20 10:48, Patrick Georgi wrote:
Nico Huber via coreboot coreboot@coreboot.org schrieb am So., 26. Jan. 2020, 02:07:
I've recently seen how much trouble it can cause when a whole new platform directory isn't hooked up for build tests. One has to double check every patch touching a file there. If in doubt, even run manual tests. That's a huge burden not only during development but also for reviewers. A burden that Jenkins would be happy to take if we let him. And even if people try to take care, they're just people, the untested code will break eventually.
Of course, there'll always be a gap when a new platform is added. We could make it a rule, though, that no commit should be merged to the master branch, before the one that hooks the build test up is reviewed.
This means that such code isn't build tested for even longer, effectively the whole development cycle until all the ducks are lined up.
you mean it's less tested when it's sitting in the review queue, waiting for the hook-up? I don't see any difference in that aspect. As long as it's not hooked up for Jenkins, only the developers that work on the new platform will do build tests. And they can only spot conflicts when/ after rebasing the patch train, no matter if parts of it are already merged. However, in case of conflicts, one could update patches that aren't merged yet. But for patches that are already merged, one would have to write fixups (more patches) and would have room to repeat the error (I feel like I've seen fixups for fixups before).
Then, if anything goes wrong, and it's still not hooked up after 24h,
revert it.
How about that?
How about this: we allow utterly incomplete mainboards in, if they're hidden behind some flag. Abuild and Jenkins build these, regular users don't even see them.
The "utterly" aside, that's already common practice. Just that the mainboards aren't hidden. And I don't see a problem with that. In case of new platforms, "users" aren't expected to have the boards anyway.
Maybe even in a designated area within src/mainboard? "Staging" perhaps?
Probably overkill, how about simply having a warning instead of the board name in the Kconfig prompt?
We have a number of conflicting goals (having usable code presented to users, getting development into the main tree early to prevent code drops after everything is said and done, incremental changes to ease review and following what's going on) and that would provide a reasonable compromise: the unfinished code is clearly identified as such but it's there for (automated) testing and we can encourage incremental, incomplete changes there.
Ack.
Nico