Here’s my mental model of the commit status state space, which is actually two dimensional (review state and dependency state).
Primer on how we develop
In SourceCred, we develop at the level of semantically atomic commits (see CONTRIBUTING.md). This means that for a particular feature, we might have 10+ small commits, each of which makes a small, easily understood change to the codebase.
As a review+merge workflow, we put every commit into its own branch, and then create a sequence of PRs merging the small commits together. Like:
PR1: Merge branch has-commit-1 into master
PR2: Merge branch has-commit-2 into has-commit-1
PR3: Merge branch has-commit-3 into has-commit-2
Once every PR in the sequence is approved, we merge them together, with the following steps:
Retarget PR2 to merge to master
Retarget PR3 to merge to master
In this example, we need to finish reviewing PR3 before we will merge PR1 or PR2. Hence, we tend to have an accumulation of PRs, and we need some way to organize + understand where they’re at, at a glance.
Understanding the state space
Rather than proposing a set of labels, I think we should try to first understand the (abstract) state space that commits are embedded in. Then, if we decide adding labels is the right approach, we can define a mapping from states to GitHub labels.
This commit is waiting for a code review. (Maybe it hasn’t been reviewed, or maybe it’s been reviewed but still needs feedback from a particular domain expert.) Generally, the commit should enter this state whenever its code changes.
This commit has been reviewed, and the reviewer wants some changes to the underlying commit.
This commit code has been reviewed and approved by all needed reviewers.
This commit can’t yet merge, because its parent commit(s) aren’t yet ready. For example, they may not be approved yet.
This commit can’t yet merge, because its children commit(s) aren’t yet ready. For example, we may still be figuring out exactly how the features that depend on this commit will work, and we may want to tweak this commit as we discover new constraints.
This commit is ready. This means that every commit in this commit’s scope (parents and children) are approved and are also ready. At this point, we can merge this sequence of commits (starting with the oldest parent and proceeding one-by-one to the youngest child).
We could come up with GitHub PR labels, but I think it will be awkward. Managing state transitions by hand will be hard and the PRs will often be in the wrong state. We’ll need to invest in training folks to update them faithfully. Also many contributors may not have the right permissions to change the labels themselves.
(It’s also awkward because are development process operates at the level of commits, not PRs.)
Actions not Labels?
Could we instead use GitHub actions to implement a review process like the one we want? The copy for GitHub actions says:
Make code reviews, branch management, and issue triaging work the way you want.
If we can implement an automated code review management system that matches our workflow, that would be far better than a system of labels. @anon60584824 I know you’ve looked into GitHub actions; do you think this is possible?
I think for the actions to really work, we would need actions to have a first-class concept of a “commit chain” or (heh) “commit branch”. A “commit chain” is an ordered sequence of commits.
Then, the “Readiness State” from shifts from being a property of the commit to being a property of the “commit chain”. A commit chain is ready if every one of its constituent commits is approved; otherwise, it is not ready.
It would be great if we could organize the UI to group PRs by commit chain, and to have a “merge commit chain” button which automates merging all the constituent commits in the right order.