On 26.01.20 10:48, Patrick Georgi wrote:
Nico Huber via coreboot <coreboot(a)coreboot.org>
schrieb am So., 26. Jan.
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,
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