[openstack-dev] [Fuel] Patch size limit
eli at mirantis.com
Tue Dec 1 16:20:04 UTC 2015
Hi Maciej, thank you for bringing this up,
+1, but we should discuss the limit, personally for me it's ok to review
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.
On Tue, Dec 1, 2015 at 3:40 PM, Maciej Kwiek <mkwiek at mirantis.com> wrote:
> 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.
> Maciej Kwiek
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the OpenStack-dev