From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 1DB42231E for ; Thu, 20 Jul 2017 11:00:29 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP; 20 Jul 2017 02:00:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,383,1496127600"; d="scan'208";a="113431592" Received: from debian-zgviawfucg.sh.intel.com (HELO debian-ZGViaWFuCg) ([10.67.104.212]) by orsmga002.jf.intel.com with ESMTP; 20 Jul 2017 02:00:27 -0700 Date: Thu, 20 Jul 2017 17:01:25 +0800 From: Tiwei Bie To: Thomas Monjalon Cc: Jens Freimann , "Van Haaren, Harry" , dev@dpdk.org Message-ID: <20170720090125.GA3807@debian-ZGViaWFuCg> References: <1500455196-182365-1-git-send-email-tiwei.bie@intel.com> <20170719102321.GA6991@debian-ZGViaWFuCg> <20170720075601.cbuizcbke5svgsos@dhcp-192-218.str.redhat.com> <1607182.kpCMdDpRBB@xps> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1607182.kpCMdDpRBB@xps> User-Agent: Mutt/1.7.2 (2016-11-26) Subject: Re: [dpdk-dev] [PATCH] all: refactor coding style 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: , X-List-Received-Date: Thu, 20 Jul 2017 09:00:30 -0000 On Thu, Jul 20, 2017 at 11:32:18AM +0300, Thomas Monjalon wrote: > 20/07/2017 10:56, Jens Freimann: > > On Wed, Jul 19, 2017 at 06:23:21PM +0800, Tiwei Bie wrote: > > >On Wed, Jul 19, 2017 at 05:24:38PM +0800, Van Haaren, Harry wrote: > > [...] > > > > Hi Tiwei, > > > > > > > > Although the idea and motivation for code-cleanup are good, performing > > > > large cleanup across a code-base is not a good solution. The reason that > > > > these types of cleanups (or even re-formatting the entire codebase) are not > > > > performed often is that it "invalidates" any currently-in-progress patch-sets. > > > > As a result, more work is required from many contributors to rebase useful > > > > features due to across-the-board white-space cleanups. > > > > > > > > Just expressing concern that we need to think carefully about the impacts > > > > of such a patch. > > > > > > > > > > Yeah, I agree. Such patch may cause many conflicts. But this patch > > > is almost generated automatically, that is to say, it's a quick work. > > > And it's more like some fixes (for the bad coding style) rather than > > > silly re-formatting done by `indent'. So I just want to share it with > > > the community, and see the potential feedbacks. Thank you for your > > > comments! :) > > > > what I'm more concerned about with these kind of huge clean-ups is > > that it makes git-blame less useful for me. Next time I want to look > > up who changed this line I'll just find your cleanup patch. Then I have > > to do another step to find out which commit introduced the change I'm > > looking for. > > > > I'm more for cleaning up these things next time you do a semantic > > change in this code. > > +1 for doing clean-up when refactoring code Hi Jens and Thomas, I agree with your concerns. But if you look into this patch, you will find that it's not a huge cleanup. Actually in this patch, although the file list is long, there are only 69 lines are changed (across the whole DPDK source code), and only 1 new line is added. The changes in each file are very minimal. I don't think it will destroy the useful info you need in git-blame. I definitely agree that it would be perfect to clean-up the code when you need to do a semantic change in this code. But you will also find that it's very possible that you won't need to do a semantic change to these code for a very long time. And much of the changed code in this patch is old code. I think the new code for DPDK has much better quality than before. It's really annoying (at least to me) each time come across those bad code. :-( Thank you for sharing your thoughts! :-) Best regards, Tiwei Bie