From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 57857A0613 for ; Thu, 29 Aug 2019 20:36:02 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id CDBC41E534; Thu, 29 Aug 2019 20:36:00 +0200 (CEST) Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by dpdk.org (Postfix) with ESMTP id 48AFC1DFE8 for ; Thu, 29 Aug 2019 20:35:59 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id B114D471; Thu, 29 Aug 2019 14:35:57 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Thu, 29 Aug 2019 14:35:58 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=OGpNorn8oXikxvQwG99StTF7UY9w78yXKh57mX6z5hE=; b=r/9Z0AK7izYp r7/OanjSLeT14ZB/tIQjRdxjbHj53/mihr1RyVl7zmbkPdgrJE4qJF2jzfzt896A HcMCrmKPYLrhnN/yniPpv/SULWf7hemA2Xd2ai2PkgizAGhTt2eH7q5VFSicuivX M2JQlMr0vDRF5wVAxarldUmqjFIakGs= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=OGpNorn8oXikxvQwG99StTF7UY9w78yXKh57mX6z5 hE=; b=zBO5SCGyqlCS97H5Oa1/vDzZMvF6yHt97hcghSkSENbv6ea7wpTxpcpSE XGCEMMYgeA8x2ilUyVB9cHdV67vN0pHoeq0Bkun9OKPVhF+nGyZrHurcq9Y1k833 YbTjRor0wbyxXMc51sk0LAj6cm/K4PbSg+GXhIbM1zT6QMEMZsGZ+DYF+B4DCJQb 6Eyoj91Bj+ijpIjF59NYX77NGBQHWXaPkxNTFVGxmgcV3FFfjS41XF8/7fc+dox5 31wY3K6MeqmyraO+ugZstWiLSPvCTFGM0iy4MixkWVn3TOW1mCX1M21Mdh7SUYUn SNJiHDIGYbxEp6/RX8LcxdvO6e3UA== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduvddrudeivddguddvfecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc fkphepleefrddvfedruddthedrvddvvdenucfrrghrrghmpehmrghilhhfrhhomhepthhh ohhmrghssehmohhnjhgrlhhonhdrnhgvthenucevlhhushhtvghrufhiiigvpedt X-ME-Proxy: Received: from xps.localnet (222.105.23.93.rev.sfr.net [93.23.105.222]) by mail.messagingengine.com (Postfix) with ESMTPA id 470BC80060; Thu, 29 Aug 2019 14:35:53 -0400 (EDT) From: Thomas Monjalon To: Andrew Rybchenko , Aaron Conole Cc: Ferruh Yigit , "dev@dpdk.org" , Ivan Ilchenko Date: Thu, 29 Aug 2019 19:05:43 +0200 Message-ID: <13169824.Gphp0D0tZG@xps> In-Reply-To: <660e1152-9ae5-8698-5e1f-bca45943cfd9@solarflare.com> References: <1566915962-5472-1-git-send-email-arybchenko@solarflare.com> <660e1152-9ae5-8698-5e1f-bca45943cfd9@solarflare.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH 00/51] ethdev: change rte_eth_dev_info_get() return value to int X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 28/08/2019 16:29, Andrew Rybchenko: > On 8/28/19 4:42 PM, Aaron Conole wrote: > > Andrew Rybchenko writes: > >> On 8/27/19 11:47 PM, Aaron Conole wrote: > >>> Andrew Rybchenko writes: > >>> > >>>> It is the first patch series to get rid of void returning functions > >>>> in ethdev in accordance with deprecation notice [1]. > >>> This is a huge series, and I suggest to combine some of the work, and/or > >>> break it up. > >> I can send patches for examples separately, but it will not help a lot. > >> I can squash changes in examples, but I think it is wrong since it would > >> make review harder - different maintainers and different practices to > >> handle error in different examples (and we tried to take it into account). > > Hrrm? Not sure what you mean. > > I mean that it is easier to review many small patches than one huge patch > especially when these files are maintained by different people. > > > Patches should be broken up by logical change. That way, it is easy to > > bisect and isolate what has changed. This series, it seems like there's > > a single logical change, and that's been spread over lots of patches. > > Single huge patch is worse for both bisect and review. When patch is huge > and bisect says that the patch is guilty, you still need to find out which > snippet/change is guilty. > > In this particular case nothing prevents to split the patch make it easier > to review and bisect. > > > I think grouping all the examples and all the app/test together, would > > make the series 14 review-able patches. As it is, stepping through 40+ > > 10-line emails is much more tedious (not to mention needing to apply > > them, check each for build, etc). > > Yes, less build cycles are required for smaller number of patches, but > anyway automation does (should do) it. So, not that important. > > I disagree that it is easier to review one huge patch. Sorry. > I think it is important here that different examples are maintained > by different people. Anyway if more reviewers will support the idea > to squash examples into once patch, technically it is trial to do. When doing same kind of change on multiple applications, splitting patches won't help any bisect (they are all different applications anyway). And I think squashing can better show the idea that every applications got the same kind of change (thanks for that). In general, I think patches deserve to be split when there is something interesting to say in the commit log. If you want to describe a different logic of each app, why not. The other way is to explain some exceptions for some applications in a signle patch. My conclusion is that it is often hard to find the good balance, between split and squash, and I can understand any motivation :) Sometimes I squash some patches on apply after they got all reviewed separately. In this case, I didn't look yet :)