<div dir="ltr"><br><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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Do we have or follow a standard on patch granularity? </div></blockquote><div><br></div><div>I don't think we have or should have any strict standard about this. Every reviewer is different and prefers different kind of patch granularity.<br><br></div><div>For example I really dislike if someone break one logical work (like implement one small feature) into 10 smaller patches (all in the same area) to easy review. To me reviewing 10 smaller patches is much more work than review one bigger because I keep loosing the context and the bigger picture. In such cases I typically end up downloading the series locally and review in IDE. But than I loose the gerrit features like compare patchsets.<br><br></div><div>Others, on the other hand, prefer to see tiny patches and that helps them to review.<br><br></div><div>Ultimately, if you want to get a patch in, you need to know the culture and the people to know how to shape it.<br></div><div>A rule of thumb: in VDSM break up the patches to as small as possible, in engine, break up into logical areas of expertise (e.g. DB, core, frontend). <br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">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. </div></blockquote><div><br></div><div>>From my perspective this is a complete waste of time. It does not bring any more clarity to the review nor to the git log. It may be aesthetically more appealing but brings much more work while babysitting the patches and does not bring anything of value. Also I feel it discourages people to fix issues like typo in comment when crossing it because they will have to create a separate patch, than either depend on it or have conflicts with it. <br><br>I personally prefer to follow the boy scout rule. On the other hand, some find it disturbing. That is fine, we are all humans, there is no one good option and this is not a technical question...<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Others don't seem to mind. I feel like I always want to ask, but I feel bad because it's a hassle.</div></blockquote><div><br><br>Contributing code is a way of communication with people. I don't think asking anything is a problem...<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><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="gmail-HOEnZb"><font color="#888888"><div><br></div><div><div><div><div>-- <br><div class="gmail-m_6997163470559330866gmail_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></div>