<div dir="ltr">Hi Maciej, thank you for bringing this up,<div><br></div><div>+1, but we should discuss the limit, personally for me it's ok to review 400loc patches,</div><div>if the patch covers only one bug-fix/feature implementation.</div><div><br></div><div>So if everybody is agree, we should:</div><div>1. update contribution guide</div><div>2. create a task for *non-voting* gate, which will -1 patch if it has loc >= x (400?),</div><div>    also as Neil mentioned, it shouldn't take into account files renaming.</div><div><br></div><div>Thanks,</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Dec 1, 2015 at 3:40 PM, Maciej Kwiek <span dir="ltr"><<a href="mailto:mkwiek@mirantis.com" target="_blank">mkwiek@mirantis.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">Hi,<div><br></div><div>I recently noticed the influx of big patches hitting Gerrit (especially in fuel-web, but I also heard that there was a couple of big ones in library). I think that patches that have 1000 LOC are simply too big to review thoroughly and reliably.</div><div><br></div><div>I would argue that there should be a limit to patch size, either a soft one (i.e. written down in contributor guidelines) or a hard one (e.g. enforced by gate job).</div><div><br></div><div>I think that we need a discussion whether we really need this limit, what should it be, and how to enforce it.</div><div><br></div><div>I personally think that most patches that are over 400 LOC could be easily split into at least two smaller changes.</div><div><br></div><div>Regarding the limit enforcement - I think we should go with the soft limit, with X LOC written as a guideline and giving the reviewers a possibility to give -1s to patches that are too big, but also giving the possibility to merge bigger changes if it's absolutely necessary (in really rare cases the changes simply cannot be split). We may mix in the hard limit for ridiculously large patches (twice the "soft limit" would be good in my opinion), so the gate would automatically reject such patches, forcing contributor to split his patch.</div><div><br></div><div>Please share your thoughts on this.</div><div><br></div><div>Regards,</div><div>Maciej Kwiek</div></div>
<br>__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div><br></div>