| Message ID | 20220511032659.641834-1-jiawenwu@trustnetic.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Wangxun 10 Gigabit Ethernet Driver | expand |
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.
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.
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
> By the way, I have some email related questions. > I can't find the message of my reply in Patchwork. So I don't know whether > my reply has been sent successfully. Try looking at the other archive, like lore. It could be you are obfuscating your email in MIME, so it got dropped. > Also, I always receive two identical reply emails from other people at the > same time. Are there settings I have forgotten? That is probably one direct and one via the list. Take a look in the email header to confirm this, look at the path the email took. I actually find this useful, since i have all the list emails going into a separate mailbox. Replies from the list can be missed, since they are just one among 700 a day, but they do form a good local archive. The direct emails land in my main mailbox, where they get seen quickly. Andrew