-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/04/2015 10:07 AM, Martin Roth wrote:
As others have stated, my concern is the tradeoff between long and short times. Too short a time, and people don't getting the chance to review patches. Too long a time and it delays development, frustrates developers, and requires rebasing the patch, maybe requiring MORE review.
Alternate thoughts or proposals:
- "Note that some contributors prefer to work on coreboot during
business hours, so consider leaving patches up longer than 24 hours during weekends and holidays" This of course presents other issues of dealing with holidays in various countries, time zones, etc.
- Trivial patches (Whitespace, spelling, and the like) can be
submitted with no delay. Non-trivial commits touching fewer than 20 lines of code should be submitted no sooner than 24 hours after the last major change. Changes larger than that 20 lines should wait at least 48? 72? hours after the last major change.
Paul also mentioned that a method of getting non-trivial patches in quickly should be addressed as well. This should not be a regular occurrence, but sometimes we need or desire to push a patch quickly. He suggested 2 +2s and a statement to the mailing list. To me, this defeats the purpose of quick.
Yesterday tpearson requested on IRC that we get a patch in quickly. It was, in my opinion significant, although just a single line change, which made it easy to understand and review. We got 3 +2s on it in just a few minutes. I thought that was a good barrier and would like to propose that as the rule: If you can get 3 +2s, knowing that it's going to be submitted immediately, it can be submitted. If this gets abused, it can be reviewed and changed at some point in the future.
Martin
Yes, this is a good compromise. There isn't very much we can do about latency on non-emergency patches; however due to the increased delays per-patch, patch trains will become the dominant method of submitting large changes (I am expecting a variant of Little's Law to hold here). As a result, the improved patch train handling set out in the CL becomes critical. I assume all reviewers are on board with the idea of not rebasing unless something is broken, and handling formatting or other non-functional changes at the end of a series?
Thanks!
- -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 (direct line) +1 (512) 690-0200 (switchboard) http://www.raptorengineeringinc.com