Neil, just to clarify: moved/renamed files are marked as "R" so I think there may be some way to ignore such files when counting LOC.
Maciej, I completely agree with you. It's pretty hard to review such big change,and takes a lot of time which could be saved by submitting smaller patches. +1 On Tue, Dec 1, 2015 at 1:50 PM, Neil Jerram <neil.jer...@metaswitch.com> wrote: > On 01/12/15 12:45, Maciej Kwiek 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. > > I think most of your principle is correct. However I can imagine a file > renaming / moving patch that would appear in Gerrit to be >=1000 LOC, > but would actually just be file moves, with perhaps some trivial changes > to Python module paths; and I don't think it would be helpful to force a > patch like that to be split up. So it may not be correct to enforce a > hard limit all the time. > > Neil > > > __________________________________________________________________________ > 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 > -- *Sylwester Brzeczkowski* Junior Python Software Engineer Product Development-Core : Product Engineering
__________________________________________________________________________ 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
