Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43484 )
Change subject: Documentation: Revise "24 hours wait period" rules ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/43484/1/Documentation/getting_start... File Documentation/getting_started/gerrit_guidelines.md:
https://review.coreboot.org/c/coreboot/+/43484/1/Documentation/getting_start... PS1, Line 64: TIMELESS
One could store the artifacts' SHA256 hashes for each commit somewhere, then compare. […]
To keep the hashes somewhere, we'd have to do TIMELESS builds on jenkins in the first place (their hashes are different from regular builds) which doubles the build effort. And it's down the rabbit hole from there.
https://review.coreboot.org/c/coreboot/+/43484/2/Documentation/getting_start... File Documentation/getting_started/gerrit_guidelines.md:
https://review.coreboot.org/c/coreboot/+/43484/2/Documentation/getting_start... PS2, Line 53: recently introduced
works as an adjective: recently-introduced
Ack
https://review.coreboot.org/c/coreboot/+/43484/2/Documentation/getting_start... PS2, Line 56: The change itself should be as limited in scope : and impact as possible to make it simple to assess the impact.
I just thought about it for a while, and I noticed a certain pattern: fixup commits work best for bu […]
To be honest, this section is already longer than I'm comfortable with. It may be useful to add something like this (but ideally without calling out individual commits because that can seem to call out their developers and they also require context that will fade over time) as a "strategies / how to" section later in the document. Are you up for it?
https://review.coreboot.org/c/coreboot/+/43484/2/Documentation/getting_start... PS2, Line 61: * Trivial changes that deal with minor issues like inconsistencies in
Mention that such changes need to be XS? (or maybe S, but that would be a bit of a stretch)
I defused the entire "trivial change" exception by noting that if they're trivial enough they can also wait. That way this rule is about submitters seeing a change and wanting to get them in because they're obviously trivial and not be bothered about it _again_ (sometimes next day), and not about a right the patch editor can claim.
A change that replaces 100 uses of parralel with parallel might be trivial even if it's not "XS" or even "S" anymore. It still should be simple to follow that there's no adverse impact (hence: timeless builds).
https://review.coreboot.org/c/coreboot/+/43484/2/Documentation/getting_start... PS2, Line 63: should point out in their commit : messages that they're trivial
I've got mixed feelings about this. […]
Right, for trivial changes it's more reasonable to have the submitter state that assessment in a comment (also gives an implicit four-eyes review on the question of triviality). Done.