Hi Maciej, thank you for bringing this up, +1, but we should discuss the limit, personally for me it's ok to review 400loc patches, if the patch covers only one bug-fix/feature implementation.
So if everybody is agree, we should: 1. update contribution guide 2. create a task for *non-voting* gate, which will -1 patch if it has loc >= x (400?), also as Neil mentioned, it shouldn't take into account files renaming. Thanks, On Tue, Dec 1, 2015 at 3:40 PM, Maciej Kwiek <mkw...@mirantis.com> wrote: > Hi, > > 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. > > 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). > > I think that we need a discussion whether we really need this limit, what > should it be, and how to enforce it. > > I personally think that most patches that are over 400 LOC could be easily > split into at least two smaller changes. > > 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. > > Please share your thoughts on this. > > Regards, > Maciej Kwiek > > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > >
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev