DPDK patches and discussions
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: "Wiles, Keith" <keith.wiles@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] tools brainstorming
Date: Thu, 9 Apr 2015 14:23:49 -0700	[thread overview]
Message-ID: <20150409142349.5b0b3293@urahara> (raw)
In-Reply-To: <D14C51E1.1AA2A%keith.wiles@intel.com>

On Thu, 9 Apr 2015 21:10:19 +0000
"Wiles, Keith" <keith.wiles@intel.com> wrote:

> 
> 
> On 4/9/15, 2:38 PM, "Jay Rolette" <rolette@infiniteio.com> wrote:
> 
> >On Thu, Apr 9, 2015 at 2:16 PM, Neil Horman <nhorman@tuxdriver.com> 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 <rolette@infiniteio.com> 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 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.
> 
> I looked at checkpatch.pl for few minutes and the code does check for C99
> comments and adding a command line option to allow C99 comments could
> pretty simple. I found the code around line 3048 or search for C99, it is
> possible it could accepted back into Linux as long as the default option
> was to not allow C99 comments.
> 
> Allowing C99 comments would be nice and the only problem I could see if
> some compiler has a problem with them. I believe all of the compilers we
> support allow C99 comments.
> 
> The only other reason to allow them is if we add some open source code in
> the future to DPDK which has C99 comments and if would be a pain to have
> to convert that code every time the open source group released a new
> version. It does open that path IMO.
> 
> Regards,
> ++Keith
> >
> 

But forking a tool means maintaining that tool.

  reply	other threads:[~2015-04-09 21:23 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-20 14:51 Thomas Monjalon
2015-03-20 15:07 ` Butler, Siobhan A
2015-03-23 16:18   ` Thomas Monjalon
2015-03-23 16:50     ` Butler, Siobhan A
2015-03-23 17:35     ` Neil Horman
2015-03-23 23:38     ` Matthew Hall
2015-03-20 15:16 ` Neil Horman
2015-03-23 16:22   ` Jim Thompson
2015-03-23 17:44     ` Neil Horman
2015-03-23 21:56       ` Jim Thompson
2015-03-23 23:01         ` Neil Horman
2015-03-23 16:26   ` Thomas Monjalon
2015-03-20 15:18 ` Simon Kågström
2015-03-23 16:29   ` Thomas Monjalon
2015-03-24  8:31     ` Simon Kågström
2015-03-23  8:41 ` Cao, Waterman
2015-03-23 16:18 ` Mcnamara, John
2015-04-08 10:43 ` Butler, Siobhan A
2015-04-08 11:43   ` Neil Horman
2015-04-08 12:16     ` Butler, Siobhan A
2015-04-08 12:20       ` Butler, Siobhan A
2015-04-08 13:11       ` Neil Horman
2015-04-08 14:40         ` Butler, Siobhan A
2015-04-08 15:39           ` Neil Horman
2015-04-08 22:29           ` Jay Rolette
2015-04-08 22:38             ` Stephen Hemminger
2015-04-09 16:31               ` Jay Rolette
2015-04-09 19:16                 ` Neil Horman
2015-04-09 19:38                   ` Jay Rolette
2015-04-09 20:14                     ` Neil Horman
2015-04-09 21:10                     ` Wiles, Keith
2015-04-09 21:23                       ` Stephen Hemminger [this message]
2015-04-09 21:29                         ` Wiles, Keith
2015-04-10  0:16                           ` Neil Horman
2015-04-10  0:26                       ` Neil Horman
2015-04-10  1:49                         ` Wiles, Keith
2015-04-10 11:41                           ` Neil Horman
2015-04-10 14:43                             ` Wiles, Keith
2015-04-08 14:16   ` Wiles, Keith
2015-04-14 14:50     ` Bruce Richardson
2015-04-08 15:21   ` Wiles, Keith
2015-04-08 15:53   ` Wiles, Keith
2015-04-08 16:16     ` Thomas Monjalon
2015-04-08 16:25       ` Wiles, Keith
2015-04-08 19:54       ` Butler, Siobhan A
2015-04-14 14:21         ` Bruce Richardson
2015-04-14 14:38           ` Neil Horman
2015-04-14 14:47             ` Thomas Monjalon
2015-04-14 14:54               ` Bruce Richardson
2015-04-14 14:52       ` Bruce Richardson
2015-04-14 15:24         ` Thomas Monjalon
2015-04-14 16:19           ` Wiles, Keith
2015-04-14 18:52             ` Wiles, Keith
2015-04-08 18:16   ` Stephen Hemminger
2015-04-08 18:58     ` Matthew Hall
2015-04-08 22:12       ` Stephen Hemminger
2015-04-08 19:51     ` Butler, Siobhan A
2015-04-14 15:29     ` Bruce Richardson
2015-04-08 21:55   ` Don Provan
2015-04-13 15:02   ` Neil Horman
2015-04-13 23:44     ` Stephen Hemminger
2015-04-16 10:49   ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150409142349.5b0b3293@urahara \
    --to=stephen@networkplumber.org \
    --cc=dev@dpdk.org \
    --cc=keith.wiles@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).