From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id D26CA2BC3 for ; Wed, 20 Apr 2016 13:02:32 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP; 20 Apr 2016 04:02:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,509,1455004800"; d="scan'208";a="962628896" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.220.132]) by fmsmga002.fm.intel.com with SMTP; 20 Apr 2016 04:01:09 -0700 Received: by (sSMTP sendmail emulation); Wed, 20 Apr 2016 12:01:07 +0025 Date: Wed, 20 Apr 2016 12:01:07 +0100 From: Bruce Richardson To: Stephen Hurd Cc: dev@dpdk.org, Thomas Monjalon Message-ID: <20160420110107.GA11120@bricha3-MOBL3> References: <1456978137-98097-1-git-send-email-stephen.hurd@broadcom.com> <1457125528-128877-4-git-send-email-stephen.hurd@broadcom.com> <20160419141933.GC15456@bricha3-MOBL3> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v3 3/7] drivers/net/bnxt new driver for Broadcom bnxt X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Apr 2016 11:02:33 -0000 On Tue, Apr 19, 2016 at 01:51:32PM -0700, Stephen Hurd wrote: > On Tue, Apr 19, 2016 at 7:19 AM, Bruce Richardson < > bruce.richardson@intel.com> wrote: > > > On Fri, Mar 04, 2016 at 01:05:24PM -0800, Stephen Hurd wrote: > > > New driver for Broadcom bnxt (NexXtreme C-series) devices. > > > > This seems a single huge commit. Can this be split up into separate commits > > with self-contained changes in each one, e.g. a feature per commit, > > starting > > with basic init, then RX and TX etc. etc.? > > > > I suppose it could, I'm not sure what the value of that approach would be > though... it would just be a long process of copy/paste/commit/repeat. > Internally, it was developed as a port/rewrite of another driver, so we > don't actually have history without a large commit. > > Assuming each individual commit needs to be buildable, this will end up > burning a lot of time, and I doubt each separate commit could be reasonably > tested. > > It's not for testing, more for code review and to help understand the code [though as you say, we do need to ensure that each commit doesn't actually break the build]. Right now, the driver code goes in as a single commit - which makes it a hard enough task to review and see what is in there. One suggestion that hopefully wouldn't be too much work might be to split the code up into: basic device init code, RX and TX functions, and then any additional features based on top of that [ideally one patch per added feature]. Regards, /Bruce