DPDK patches and discussions
 help / color / mirror / Atom feed
From: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
To: "Wiles, Keith" <keith.wiles@intel.com>
Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [dpdk-dev] [PATCH v1 2/3] net/hyperv: implement core functionality
Date: Tue, 19 Dec 2017 09:25:36 +0100	[thread overview]
Message-ID: <20171219082536.wojt26lpzft6fdof@laranjeiro-vm.dev.6wind.com> (raw)
In-Reply-To: <FF653BDF-EE70-465F-AD1C-C88067521E8E@intel.com>

Hi Keith,

On Mon, Dec 18, 2017 at 06:43:35PM +0000, Wiles, Keith wrote:
> 
> 
> > On Dec 18, 2017, at 11:59 AM, Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote:
> 
> >> Not to criticize style, but a few blank lines could help in
> >> readability for these files IMHO. Unless blank lines are illegal
> >> :-)
> > 
> > It's a matter of taste, I think people tend to add random blank lines where
> > they think doing so clarifies things for themselves, resulting in
> > inconsistent coding style not much clearer for everyone after several
> > iterations.
> > 
> > As a maintainer I've grown tired of discussions related to blank lines while
> > reviewing patches. That's why except for a few special cases, I now enforce
> > exactly the bare minimum of one blank line between variable declarations and
> > the rest of the code inside each block.
> > 
> > If doing so makes a function unreadable then perhaps it needs to be split :)
> > I'm sure you'll understand!
> 
> I do not really understand the problem as I have not seen any
> complaints about blank lines unless two or more in a row. I have never
> seen someone complain about a given blank line in a function, unless a
> missing one to split up the declared variables and code in a function
> or block of code.

It is true when the amount of blank lines are few and logical, but we
generally see patch where in the same file we see random blank lines
added without any logic, generally to easily identify where the
modification are done.

> It is a shame you have decided to take the minimum approach to blank
> lines, IMO it does not make a lot of sense. I only bring it up to help
> others with reading your code like our customers.
>
> We do not have rule for this so I can not force anyone to add blank
> lines for readability, so I have to live with it. :-(

As there is no clear rules, the best one is limiting this situation to
the extreme minimal, otherwise explaining the logic behind it is very
difficult as it will differ from one maintainer to another one, it will
increase the amount of patches refused due to coding style issues.

> > Regards,
> > 
> > -- 
> > Adrien Mazarguil
> > 6WIND
> 
> Regards,
> Keith
> 

Regards,

-- 
Nélio Laranjeiro
6WIND

  reply	other threads:[~2017-12-19  8:26 UTC|newest]

Thread overview: 112+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20171124160801.GU4062@6wind.com>
     [not found] ` <20171124164812.GV4062@6wind.com>
2017-11-24 17:21   ` [dpdk-dev] [RFC] Introduce virtual PMD for Hyper-V/Azure platforms Adrien Mazarguil
2017-12-18 16:46     ` [dpdk-dev] [PATCH v1 0/3] " Adrien Mazarguil
2017-12-18 16:46       ` [dpdk-dev] [PATCH v1 1/3] net/hyperv: introduce MS Hyper-V platform driver Adrien Mazarguil
2017-12-18 18:28         ` Stephen Hemminger
2017-12-18 19:54           ` Thomas Monjalon
2017-12-18 21:17             ` Stephen Hemminger
2017-12-19 10:01               ` Adrien Mazarguil
2017-12-19 11:15                 ` Thomas Monjalon
2017-12-19 13:13                   ` Adrien Mazarguil
2017-12-18 16:46       ` [dpdk-dev] [PATCH v1 2/3] net/hyperv: implement core functionality Adrien Mazarguil
2017-12-18 17:04         ` Wiles, Keith
2017-12-18 17:59           ` Adrien Mazarguil
2017-12-18 18:43             ` Wiles, Keith
2017-12-19  8:25               ` Nelio Laranjeiro [this message]
2017-12-18 18:26         ` Stephen Hemminger
2017-12-18 20:21           ` Adrien Mazarguil
2017-12-18 21:03             ` Thomas Monjalon
2017-12-18 21:19               ` Stephen Hemminger
2017-12-18 18:34         ` Stephen Hemminger
2017-12-18 20:23           ` Adrien Mazarguil
2017-12-19  9:53             ` Bruce Richardson
2017-12-19 10:15               ` Adrien Mazarguil
2017-12-19 15:31                 ` Stephen Hemminger
2017-12-18 23:59         ` Stephen Hemminger
2017-12-19 10:01           ` Adrien Mazarguil
2017-12-19 15:37             ` Stephen Hemminger
2017-12-19  1:54         ` Ferruh Yigit
2017-12-19 15:06           ` Adrien Mazarguil
2017-12-19 20:44             ` Ferruh Yigit
2017-12-20 14:13               ` Thomas Monjalon
2017-12-21 16:19               ` Adrien Mazarguil
2017-12-18 16:46       ` [dpdk-dev] [PATCH v1 3/3] net/hyperv: add "force" parameter Adrien Mazarguil
2017-12-18 18:23       ` [dpdk-dev] [PATCH v1 0/3] Introduce virtual PMD for Hyper-V/Azure platforms Stephen Hemminger
2017-12-18 20:13         ` Thomas Monjalon
2017-12-19  0:40           ` Stephen Hemminger
2017-12-18 20:21         ` Adrien Mazarguil
2017-12-22 18:01       ` [dpdk-dev] [PATCH v2 0/5] " Adrien Mazarguil
2017-12-22 18:01         ` [dpdk-dev] [PATCH v2 1/5] net/failsafe: fix invalid free Adrien Mazarguil
2017-12-22 18:01         ` [dpdk-dev] [PATCH v2 2/5] net/failsafe: add "fd" parameter Adrien Mazarguil
2017-12-22 18:01         ` [dpdk-dev] [PATCH v2 3/5] net/vdev_netvsc: introduce Hyper-V platform driver Adrien Mazarguil
2017-12-22 18:01         ` [dpdk-dev] [PATCH v2 4/5] net/vdev_netvsc: implement core functionality Adrien Mazarguil
2017-12-22 18:01         ` [dpdk-dev] [PATCH v2 5/5] net/vdev_netvsc: add "force" parameter Adrien Mazarguil
2017-12-23  2:06         ` [dpdk-dev] [PATCH v2 0/5] Introduce virtual PMD for Hyper-V/Azure platforms Stephen Hemminger
2017-12-23 14:28           ` Thomas Monjalon
2018-01-09 14:47         ` [dpdk-dev] [PATCH v3 0/8] Introduce virtual driver " Matan Azrad
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 1/8] net/failsafe: fix invalid free Matan Azrad
2018-01-16 10:24             ` Gaëtan Rivet
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 2/8] net/failsafe: add "fd" parameter Matan Azrad
2018-01-16 10:54             ` Gaëtan Rivet
2018-01-16 11:19               ` Gaëtan Rivet
2018-01-16 16:17                 ` Matan Azrad
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 3/8] net/failsafe: support probed sub-devices getting Matan Azrad
2018-01-16 11:09             ` Gaëtan Rivet
2018-01-16 12:27               ` Matan Azrad
2018-01-16 14:40                 ` Gaëtan Rivet
2018-01-16 16:15                   ` Matan Azrad
2018-01-16 16:54                     ` Gaëtan Rivet
2018-01-16 17:20                       ` Matan Azrad
2018-01-16 22:31                         ` Gaëtan Rivet
2018-01-17  8:40                           ` Matan Azrad
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 4/8] net/vdev_netvsc: introduce Hyper-V platform driver Matan Azrad
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 5/8] net/vdev_netvsc: implement core functionality Matan Azrad
2018-01-09 18:49             ` Stephen Hemminger
2018-01-10 15:02               ` Matan Azrad
2018-01-17 16:51                 ` Thomas Monjalon
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 6/8] net/vdev_netvsc: skip routed netvsc probing Matan Azrad
2018-01-09 18:51             ` Stephen Hemminger
2018-01-10 15:07               ` Matan Azrad
2018-01-10 16:43                 ` Stephen Hemminger
2018-01-11  9:00                   ` Matan Azrad
2018-01-17 16:59                     ` Thomas Monjalon
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 7/8] net/vdev_netvsc: add "force" parameter Matan Azrad
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 8/8] net/vdev_netvsc: add automatic probing Matan Azrad
2018-01-18  8:43           ` [dpdk-dev] [PATCH v4 0/8] Introduce virtual driver for Hyper-V/Azure platforms Matan Azrad
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 1/8] net/failsafe: fix invalid free Matan Azrad
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 2/8] net/failsafe: add "fd" parameter Matan Azrad
2018-01-18  8:51               ` Gaëtan Rivet
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 3/8] net/failsafe: add probed etherdev capture Matan Azrad
2018-01-18  9:10               ` Gaëtan Rivet
2018-01-18  9:33                 ` Matan Azrad
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 4/8] net/vdev_netvsc: introduce Hyper-V platform driver Matan Azrad
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 5/8] net/vdev_netvsc: implement core functionality Matan Azrad
2018-01-18 18:25               ` Stephen Hemminger
2018-01-18 18:28                 ` Matan Azrad
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 6/8] net/vdev_netvsc: skip routed netvsc probing Matan Azrad
2018-01-18 18:26               ` Stephen Hemminger
2018-01-18 18:47                 ` Thomas Monjalon
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 7/8] net/vdev_netvsc: add "force" parameter Matan Azrad
2018-01-18 18:27               ` Stephen Hemminger
2018-01-18 18:30                 ` Matan Azrad
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 8/8] net/vdev_netvsc: add automatic probing Matan Azrad
2018-01-18 10:01             ` [dpdk-dev] [PATCH v5 0/8] Introduce virtual driver for Hyper-V/Azure platforms Matan Azrad
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 1/8] net/failsafe: fix invalid free Matan Azrad
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 2/8] net/failsafe: add "fd" parameter Matan Azrad
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 3/8] net/failsafe: add probed etherdev capture Matan Azrad
2018-01-18 10:08                 ` Gaëtan Rivet
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 4/8] net/vdev_netvsc: introduce Hyper-V platform driver Matan Azrad
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 5/8] net/vdev_netvsc: implement core functionality Matan Azrad
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 6/8] net/vdev_netvsc: skip routed netvsc probing Matan Azrad
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 7/8] net/vdev_netvsc: add "force" parameter Matan Azrad
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 8/8] net/vdev_netvsc: add automatic probing Matan Azrad
2018-01-18 13:51               ` [dpdk-dev] [PATCH v6 0/8] Introduce virtual driver for Hyper-V/Azure platforms Matan Azrad
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 1/8] net/failsafe: fix invalid free Matan Azrad
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 2/8] net/failsafe: add "fd" parameter Matan Azrad
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 3/8] net/failsafe: add probed etherdev capture Matan Azrad
2018-01-18 22:34                   ` Thomas Monjalon
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 4/8] net/vdev_netvsc: introduce Hyper-V platform driver Matan Azrad
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 5/8] net/vdev_netvsc: implement core functionality Matan Azrad
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 6/8] net/vdev_netvsc: skip routed netvsc probing Matan Azrad
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 7/8] net/vdev_netvsc: add "force" parameter Matan Azrad
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 8/8] net/vdev_netvsc: add automatic probing Matan Azrad
2018-01-20  1:15                 ` [dpdk-dev] [PATCH v6 0/8] Introduce virtual driver for Hyper-V/Azure platforms Ferruh Yigit

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=20171219082536.wojt26lpzft6fdof@laranjeiro-vm.dev.6wind.com \
    --to=nelio.laranjeiro@6wind.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=keith.wiles@intel.com \
    --cc=stephen@networkplumber.org \
    /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).