Open Stack

Here is a summary of the discussion we had at the last AC meeting on reviews and PRs of large, sweeping changes like runtime-rs.

This is a copy of the current state of https://etherpad.opendev.org/p/Kata_Containers_2022_Architecture_Committee_Mtgs, after review and a few iterations by at least Fabiano, Peng Tao and myself.

If there are some statements in there that you don't like or would like to change, now is the time to chime up. We can also archive that on the wiki or in the documentation if this is seen as important enough.


Thanks
Christophe


• Agreement was reached as follows:
	• Staging (runtime-rs) branch should accept things faster than main, but the code will not be considered fully reviewed until we merge it on main
	• We won't merge the staging branch on main as is
	• As necessary, more refactoring and review may be needed
	• As much as possible, we will do that incrementally, with additional PRs
	• (c3d: original text was: "without more necessary another pass of review/refactoring, PR reorganization, patch series cleanup, and so on", which I find ambiguous)
	• The goal of that staging branch is to have something that runs and can be tested quickly and allow continuous improvements, even if quality or readability is not optimal (and, even if that wasn't said in the meeting, I suspect also allows the current users of runtime-rs to keep developing, like for td-shim)
	• It would be helpful to have multiple PRs open in parallel so that we can get a big picture view of the code
	• We don't need to merge on PR before merging the next one, that presents a sequential view of the code that is not really helpful for reviewers.
	• In that sense, we are not asking for a "slow down" but for a "dump faster" ;-)
	• Like for td-shim, it would be very helpful to get documentation / PDF / presentation documents merged first, so that we understand where each PR fits
	• It would really help if PR descriptions and commit messages were more elaborate, describing:
	• What the code does in some details
	• Where it fits in the architecture
	• Deviations from the existing Go implementation (Eric gave the example of replacing multiple Go packages doing similar things with a single Rust crate)
	• External APIs at least, ideally some of the internal flow as well (notably when it's not obvious from the code itself)
	• Maturity level for the code, e.g. "this is part 1 of a 3-stage development", or "this and that won't work yet"
	• Red Hat is currently training several engineers to become proficient with Rust, but this is likely to take ~2 months to complete
	• We are expecting more RedHat developers to join the force to help reviewing and refactoring the rust shim
	• The authors are also refactoring the code as they publish it, with the objective to give us a higher quality code based on what they learned from using it in production
	• Being too busy to review is not a good reason to block a PR. Please prioritise things if a reviewer thinks it is important to review.
	• In that sense, if a reviewer blocks a PR and then disapears, we will start to dismiss some reviews after three pings (It applies to all branches not just runtime-rs).
	• c3d suggestion (not discussed during meeting): Create teams for coworkers (e.g. we have "redhat" team in kata-containers, not in CoCo yet), and make a rule that ping 2 and 3 must also ping the team, with the hope that someone close to the original commenter can pick up or relay.