Date: Thu, 03 May 2012 13:00:16 +0400 From: "Alexander V. Chernikov" <melifaro@FreeBSD.org> To: Luigi Rizzo <rizzo@iet.unipi.it> Cc: svn-src-head@FreeBSD.org, ae@FreeBSD.org, svn-src-all@FreeBSD.org, Gleb Smirnoff <glebius@FreeBSD.org>, src-committers@FreeBSD.org Subject: Re: svn commit: r234834 - in head/sys: contrib/pf/net net netinet netinet/ipfw ofed/drivers/infiniband/ulp/ipoib Message-ID: <4FA24920.4080809@FreeBSD.org> In-Reply-To: <20120430133904.GA65792@onelab2.iet.unipi.it> References: <201204301022.q3UAMNcq060049@svn.freebsd.org> <20120430114836.GA62284@onelab2.iet.unipi.it> <20120430113610.GC18777@FreeBSD.org> <4F9E82DA.4030406@FreeBSD.org> <20120430133904.GA65792@onelab2.iet.unipi.it>
next in thread | previous in thread | raw e-mail | index | archive | help
On 30.04.2012 17:39, Luigi Rizzo wrote: > On Mon, Apr 30, 2012 at 04:17:30PM +0400, Alexander V. Chernikov wrote: >> On 30.04.2012 15:36, Gleb Smirnoff wrote: >>> On Mon, Apr 30, 2012 at 01:48:36PM +0200, Luigi Rizzo wrote: >>> L> On Mon, Apr 30, 2012 at 10:22:23AM +0000, Alexander V. Chernikov wrote: >>> L> > Author: melifaro >>> L> > Date: Mon Apr 30 10:22:23 2012 >>> L> > New Revision: 234834 >>> L> > URL: http://svn.freebsd.org/changeset/base/234834 >>> L> > >>> L> > Log: >>> L> > Move several enums and structures required for L2 filtering from >>> ip_fw_private.h to ip_fw.h. >>> L> >>> L> I would be really grateful if you could revert this back and discuss >>> L> what you wanted to achieve with this change other than saving one >>> L> entry in the list of includes. >> Changing something inside ip_fw_private.h (for example, locking change) >> requires changes in several totally unrelated subsystems, which is >> clearly bad. > > There are certainly good reasons to split things even further > (as Gleb suggested) so if you want to follow that route > and have patches for review i will be glad to discuss that. Yup. I'll make another patch > > I am the first one to say that the name ip_fw_private.h is confusing, > but at the time i did not feel brave enough to move from a single > header (ip_fw.h) to the four (ip_fw.h, ip_fw_user.h, ip_fw_kernel.h, > ip_fw_kernel_private.h) that would be needed for proper confinement > of information. > > This said, the change you have made introduce a worse form of header > pollution and this is why i am requesting a backout. Reverted in r234946 > >>> L> >>> L> As clearly mentioned in the commit logs >>> L> >>> L> http://svnweb.freebsd.org/base?view=revision&revision=200580 >> Maybe there are some other possibilities documenting preferred layout >> other than commit log? Searching 2+yrs commit history is not the best >> way of finding information. > > sure, you could have asked people involved with ipfw development > to review this change. > > Also don't expect a single policy in style(9) to hold for all > possible situations: the kernel is huge, it has parts that come > from multiple origins in time and space, and sometimes is shared > with other OSs as well. Refactoring (such as your changes) should > keep that in mind. > > Let's not make a big deal of this thing: we all make mistakes or > have different opinions on how things should be done, and the only > way to avoid them is to discuss thing in advance, or be open to > resolve things when problems arise. No hard feelings. Ok :) > > cheers > luigi > -- WBR, Alexander
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4FA24920.4080809>