Attention is currently required from: Jeff Daly, Paul Menzel, Stefan Reinauer, Kyösti Mälkki. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60736 )
Change subject: util/ifdtool: Add additional regions for platforms that support them ......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
completely updated by patchset 4. still learning gerrit vs github. […]
No worries. Although https://doc.coreboot.org/getting_started/gerrit_guidelines.html describes our Gerrit guidelines, I don't think we have any introduction to Gerrit in our documentation. https://doc.coreboot.org/tutorial/part2.html covers the basics of uploading patches for review and how to update a change after it has been pushed for review, but that's it. Here are some nuggets of information that may be useful:
We don't use merges with Gerrit. I recommend enabling the `pull.rebase` Git option for the coreboot repository (as well as any other repo on Gerrit).
Gerrit has the concept of "change", which is not the same as a Git commit. A change contains a list of commits (called patchsets) and comments (for example, what you're reading right now is a comment). The patchsets in a change are like "versions" of the original commit, with the latest patchset being the most recent version.
Gerrit associates commits with a specific change using the `Change-Id` tag in the commit message. A change can be referenced by its Change-Id, and also by its "change number" (for example, CB:60736 aka https://review.coreboot.org/60736 refers to this change).
When pushing commits (to `HEAD:refs/for/<branch>`), Gerrit uses the Change-Id tag in the commit message to decide what to do: if a change with the same Change-Id already exists, the pushed commit is treated as an update (a new patchset) of the existing change; otherwise, a new change is created for the Change-Id.
It is possible to give scores to the latest patchset of a change (voting on an older patchset has no effect). Here, we have three different scores: Verified, Code-Review and All-Comments-Resolved. Verified is given by the build bot (Jenkins) after build-testing a change. Code-Review scores are given by people as part of the review process (one can still review and comment on a change without voting). All-Comments-Resolved merely reflects whether all comments have been resolved, to hopefully avoid submitting a change with unresolved comments (i.e. open discussions).
Code-Review scores are preserved after a commit message update or a rebase (but not if both things are done at the same time). A Code-Review -2 score prevents a change from being submitted and persists across patchsets. Because of this, only people in the Core Developers group can give Code-Review -2 scores, and rarely used. For example, CB:56065 got a Code-Review -2 from me because that change is fundamentally incorrect and there's nothing that can be done to fix it.
When an approved change (Verified +1, Code-Review +2, All-Comments-Resolved 👍️) is submitted, the latest patchset is cherry-picked into the corresponding branch (e.g. master). It's not possible to submit a change if the cherry-pick operation conflicts (e.g. because another commit that changes nearby areas got merged in the meantime). Because of this, approved changes might need to be manually rebased (i.e. fix the conflicts and upload a new patchset) and re-approved.
Only people in the Core Developers group are allowed to submit changes, because it's easy to accidentally "break the tree" (the master branch no longer passes the build tests) by submitting a stale change. For example, CB:58392 was made before CB:44138 got submitted, but CB:58392 was submitted after CB:44138 and required the "emergency fix" CB:58458 to unbreak the tree.
When handling patch trains (for example, your CB:60736 and CB:60737 are a patch train of two changes), I prefer to always update all changes together. Otherwise, the relation chain gets messy: the latest patchset of some changes can have an older patchset of the preceding change as parent commit. If this happens, Gerrit would show "Indirect ancestor" or "Not current" in the "Relation chain" section (top of the page, right of the box with the commit message).