Hi,
On 4/21/20 7:23 AM, Soby Mathew via TF-A wrote:
My view is that smaller patches are easier to review and we should try to break up the patches to logical chucks where possible. I haven't taken a look at the patches myself but I am sure there will be ways to break it up for ease of review.
I would like to strongly echo this. I find big patches so hard to review. There is only so much things the human brain can comprehend in one go. Smaller patches are just easier to reason about, they focus on one thing and it is easier to get your head around them because the entire patch and the interaction it may have with other components "fits" in one's head. Thus, it is much easier to reach a good level of confidence at review time.
Also, I believe there is a natural tendency of getting discouraged at the sight of big patches, smaller patches have a much better chance of getting reviewed quickly. They can also be dealt with incrementally. Say in a 10 patch stack, it may happen that the 5 first are good to go, while the sixth is more controversial and requires more discussion. In this case, being able to merge the 5 patches is a first step in the right direction.
Ideally, one should think about how to split the patches in a logical, manageable way early during development. It is true that if it is an afterthought, breaking up a huge patch down into smaller ones is a lot of work. This is why it needs doing upfront IMO.
Cheers, Sandrine
tf-a@lists.trustedfirmware.org