[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Yn2E8X6f8PJ0c4CB@lunn.ch>
Date: Fri, 13 May 2022 00:06:41 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Jiawen Wu <jiawenwu@...stnetic.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 00/14] Wangxun 10 Gigabit Ethernet Driver
On Thu, May 12, 2022 at 08:57:48AM -0700, Jakub Kicinski wrote:
> On Thu, 12 May 2022 17:43:39 +0800 Jiawen Wu wrote:
> > On Thursday, May 12, 2022 8:54 AM, Jakub Kicinski wrote:
> > > On Wed, 11 May 2022 11:26:45 +0800 Jiawen Wu wrote:
> > > > 22 files changed, 22839 insertions(+)
> > >
> > > Cut it up more, please. Expecting folks to review 23kLoC in one sitting is
> > > unrealistic. Upstream a minimal driver first then start adding features.
> >
> > I learned that the number of patches should not exceed 15 at a time, refer
> > to the guidance document.
> > May I ask your advice that the limit of one patch and the total lines?
>
> There is no strict limit, but the reality is that we have maybe
> 5 people reviewing code upstream and hundreds of developers typing
> and sending changes. So the process needs to be skewed towards making
> reviewer's life easier, reviewers are the bottleneck.
>
> So there is no easy way here. Remove as much code as possible to still
> have functional driver and cut it up. Looks like you can definitely
> drop all patches starting from patch 7 to begin with. But patches 1-6
> are still pretty huge.
Hi Jiawen Wu
After a quick look at the patches, i'm guessing you are new to
submitting to mainline. So it might even make sense to just post patch
1. Please make it standalone, so most of the txgbe.rst can be removed,
since the driver with just patch 1 has none of the features discussed
in it. We can give you feedback which should help you learn the
process, and what we expect from mainline code. You can then rework
that patch and the rest of your driver from what you have learned,
making further patches easier and faster to review.
Andrew
Powered by blists - more mailing lists