From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id AC42F5A9F for ; Thu, 9 Apr 2015 22:14:56 +0200 (CEST) Received: from hmsreliant.think-freely.org ([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1YgIqc-00043Y-QL; Thu, 09 Apr 2015 16:14:50 -0400 Date: Thu, 9 Apr 2015 16:14:46 -0400 From: Neil Horman To: Jay Rolette Message-ID: <20150409201445.GA29807@hmsreliant.think-freely.org> References: <0C5AFCA4B3408848ADF2A3073F7D8CC86D58F9C2@IRSMSX109.ger.corp.intel.com> <20150408114339.GA22959@hmsreliant.think-freely.org> <0C5AFCA4B3408848ADF2A3073F7D8CC86D58FB64@IRSMSX109.ger.corp.intel.com> <20150408131105.GD22959@hmsreliant.think-freely.org> <0C5AFCA4B3408848ADF2A3073F7D8CC86D58FDBF@IRSMSX109.ger.corp.intel.com> <0FBA33A7-A21E-426F-B44E-32E86F2B23DB@infiniteio.com> <20150408153802.2bc59227@urahara> <20150409191658.GC26201@hmsreliant.think-freely.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] tools brainstorming 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: Thu, 09 Apr 2015 20:14:57 -0000 On Thu, Apr 09, 2015 at 02:38:32PM -0500, Jay Rolette wrote: > On Thu, Apr 9, 2015 at 2:16 PM, Neil Horman wrote: > > > On Thu, Apr 09, 2015 at 11:31:39AM -0500, Jay Rolette wrote: > > > On Wed, Apr 8, 2015 at 5:38 PM, Stephen Hemminger < > > > stephen@networkplumber.org> wrote: > > > > > > > On Wed, 8 Apr 2015 16:29:54 -0600 > > > > Jay Rolette wrote: > > > > > > > > > "C comments" includes //, right? It's been part of the C standard > > for a > > > > long time now... > > > > > > > > Yes but. > > > > I like to use checkpatch and checkpatch enforces kernel style which > > does > > > > not allow // for > > > > comments. > > > > > > > > > > Fork checkpatch and disable that bit? DPDK isn't the kernel, so no > > > requirement to follow all of its rules > > > > > > > Doesn't that beg the question, why? I understand the DPDK isn't the > > kernel, but > > we're not talking about clarity of code, not anything functional to that > > code. > > It seems we would be better served by just taking something that works here > > rather than re-inventing the wheel and digging into the minuate of what > > type of > > comments should be allowed (unless there is a compelling reason to change > > it > > that supercedes the avilable tools). If not checkpath, then some other > > tool, > > but It seems to me that coding style is one of those things where we can > > bend to > > the tool rather than taking the time to make the tool do exactly whats > > desired, > > at least until someone gets the time to modify it. > > > > Fair question. > > It depends a bit on how much you want to encourage patch contributions. Is > it worth adding more pain for folks trying to contribute patches for things > like this? > > Should we force someone to spend time redoing a patch because of which way > they do their parenthesis? What about number of spaces to indent code? // > vs /* */ comments? None of these matter functionally and they don't affect > maintenance generally. > > If someone is modifying existing code, then yeah, they should follow the > prevailing style (indention level, brace alignment, etc.) of the file they > are in. It helps readability, which makes maintenance easier. However, IMO, > mixing // and /* */ for comments doesn't affect the readability of the > source. > I take your meaning (that we shouldn't be overly restrictive on aspects of the code that don't affect functionality, but I think this line of thinking quickly spirals out into the question of weather to have coding styles at all. For any aspect of code that you codify in a style guide, you are almost by definition being restrictive: // comments vs. /* * comments */ or void func(int args) { } vs. void func (int args) { } Insert your own pet coding style variants as you see fit. If we want to enforce coding styles, we have to pick something and enforce it. To suggest that we allow both (or some subset of the entire set of some coding style aspect, as I think you are trying to propose), while fine to do, somewhat calls into question the need/desire for style guidlines at all. As you note below, you're unlikely to revise a patch if the only comment is "use different commenting style". The exact same might be a response to your function declaration style, or any other aspect. If you say both/all are allowed, you quickly get to the point of not really having a style (which may be acceptible here). > I know if I submit a patch and the only feedback is that I should have used > /* */ for comments, I'm extremely unlikely spend extra time to resubmit the > patch for pedantry. Hence my desire for the tool. Ideally, style is best enforced when its a non-issue during the review process (i.e. a tool is able to tell you where your style problems are, and the issue never comes up during review). We can as you previously suggested fork a tool and modify it to conform to some other style guidelines of our own design. But honestly, I don't want to invest alot of time in what the guidelines are. i.e. I value consistency in style, not a specific style. As such, borrowing checkpatch (or some other tool), and adopting its style enforcement, seems like the best, most efficient path forward in my mind. Neil