<div dir="ltr">Hi, <div><br></div><div>I think I have found a rule which applies to lot of CRs. If you submit set of small patches, you will be asked to squash them, and if you submit 'big bang' patch, you will be asked to split it. Sometimes, and that's the hardest situation, you are asked to do both :D</div><div><br></div><div>I think you should lead with what is most effective for you to work with. And that ideally should be (for your own good) small commits with one (or some small number) logical change (rename this, add this) in local feature branch. That way you can easily remove some change if it's not needed in the end, you have patches ready for 'fine granularity reviewer', and if you need to provide big bang patch of 'bulk reviewer', you can create one in seconds. And potential 're-split' is easy too, you just squash them again differently.</div><div><br></div><div>Squashing is easy, splitting is not.</div><div><br></div><div>_warning_mine_opinion_: </div><div>About patch size in general. It's like method / class names. If it's called 'perform' and have 100 LOC, it's most probably bad. Thus if your patch description does not list all the changes and/or its description is excessively long: "I did X and Y, then Z, while W, unless Q", then your patch is big, and kinda defies purpose of VCS. Next time you should write smaller one, and squash it to bulk patch only if reviewer insists. Also patch "rename methodA->methodB" takes no time to review(eyes are really good to see same patterns), with small probablility of error, and when we all use IDE for refactorings, there's no need to review it at all. Five renames and method extractions in one patch, garnished with added (even trivial) new functionality is significantly harder to review, and reviewer really needs to pay attention to every line change.</div><div><br></div><div>M.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 29, 2017 at 6:16 PM, Greg Sheremeta <span dir="ltr"><<a href="mailto:gshereme@redhat.com" target="_blank">gshereme@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Do we have or follow a standard on patch granularity? When I started, I picked up on the culture that we do small, logical commits -- as much as possible, each commit should be focused on a specific purpose. I've perceived some reviewers prefer to have all orthogonal changes (fix a random spelling error, remove a duplicate semicolon) extracted to other patches for clarity. Others don't seem to mind. I feel like I always want to ask, but I feel bad because it's a hassle.<div><br></div><div>Also, when you are asked to extract something, do you have a trick to make it as easy as possible?<br><div><br></div><div>Best wishes,</div><div>Greg</div><span class="HOEnZb"><font color="#888888"><div><br></div><div><div><div><div>-- <br><div class="m_5226500271705080591gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Greg Sheremeta, MBA<br><span style="font-size:12.8px">Sr. Software Engineer</span><br></div><div>Red Hat, Inc.<br><a href="mailto:gshereme@redhat.com" target="_blank">gshereme@redhat.com</a><br></div></div></div></div></div>
</div></div></div></div></font></span></div></div>
<br>______________________________<wbr>_________________<br>
Devel mailing list<br>
<a href="mailto:Devel@ovirt.org">Devel@ovirt.org</a><br>
<a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/<wbr>mailman/listinfo/devel</a><br></blockquote></div><br></div>