Nico Huber via coreboot coreboot@coreboot.org schrieb am So., 26. Jan. 2020, 02:07:
Hello again,
so, we have Jenkins that runs build tests on our master branch. That makes working together on a huge project much easier. However, do we have a rule that all the code should be build tested? and if not, should we establish one?
The general rule of thumb is to build test everything in the tree. We're not doing that in a few cases, among them stuff behind default-off kconfig flags, but I think we should strive for improvement.
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.
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.
Maybe even in a designated area within src/mainboard? "Staging" perhaps?
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.
Regards, Patrick