All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: 高峰 <fgao@ikuai8.com.aqb.so>
Cc: kaber@trash.net, netfilter-devel@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	gfree.wind@gmail.com
Subject: Re: 答复: [PATCH 1/1] netfilter: Add helper array register/unregister functions
Date: Wed, 20 Jul 2016 10:50:18 +0200	[thread overview]
Message-ID: <20160720085018.GD1336@salvia> (raw)
In-Reply-To: <014e01d1e21e$062884b0$12798e10$@ikuai8.com>

On Wed, Jul 20, 2016 at 08:31:13AM +0800, 高峰 wrote:
> Thanks Pablo.
> 
> I had used the script "checkpatch.pl" to check the patch file.
> There was no indentation error reported.
> 
> So could you give me more tails please or point one indentation error?
> Then I could correct it by myself next time.

I'm refering to this specifically:

static int function(int parameter1, struct another_structure *blah,
                    int parameter2, unsigned int parameter3);
                    ^

It is just a comestic issue, but we consistently align function
parameters to the initial parens.

As I said, I have just manually fixed this here, so no problem, just
keep this in mind for the next time.

Another observation: You should bump patch version numbering in each
revision and keep some history on its evolution.

The area after the patch separator --- and before diff stats is good
to place volatile information that is only meaningful to the review
process, I mean something like this:

  subsystem: Patch title.

  Patch description...

  Signed-off-by: Lucas Skywalker <trotacielos@blackstar.org>
  ---
  v3: Address comments from Chebakia on possible backward compatibility
      issues.
  v2: New parameter to control something.
  v1: Initial patch.

  include/net/netfilter/nf_tables.h |  25 ++-
  ...
  diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h

Thanks.

WARNING: multiple messages have this Message-ID (diff)
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: 高峰 <fgao@ikuai8.com>
Cc: kaber@trash.net, netfilter-devel@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	gfree.wind@gmail.com
Subject: Re: 答复: [PATCH 1/1] netfilter: Add helper array register/unregister functions
Date: Wed, 20 Jul 2016 10:50:18 +0200	[thread overview]
Message-ID: <20160720085018.GD1336@salvia> (raw)
In-Reply-To: <014e01d1e21e$062884b0$12798e10$@ikuai8.com>

On Wed, Jul 20, 2016 at 08:31:13AM +0800, 高峰 wrote:
> Thanks Pablo.
> 
> I had used the script "checkpatch.pl" to check the patch file.
> There was no indentation error reported.
> 
> So could you give me more tails please or point one indentation error?
> Then I could correct it by myself next time.

I'm refering to this specifically:

static int function(int parameter1, struct another_structure *blah,
                    int parameter2, unsigned int parameter3);
                    ^

It is just a comestic issue, but we consistently align function
parameters to the initial parens.

As I said, I have just manually fixed this here, so no problem, just
keep this in mind for the next time.

Another observation: You should bump patch version numbering in each
revision and keep some history on its evolution.

The area after the patch separator --- and before diff stats is good
to place volatile information that is only meaningful to the review
process, I mean something like this:

  subsystem: Patch title.

  Patch description...

  Signed-off-by: Lucas Skywalker <trotacielos@blackstar.org>
  ---
  v3: Address comments from Chebakia on possible backward compatibility
      issues.
  v2: New parameter to control something.
  v1: Initial patch.

  include/net/netfilter/nf_tables.h |  25 ++-
  ...
  diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2016-07-20  8:50 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-18  3:39 [PATCH 1/1] netfilter: Add helper array register/unregister functions fgao
2016-07-18  3:39 ` fgao
2016-07-19 18:12 ` Pablo Neira Ayuso
2016-07-19 18:12   ` Pablo Neira Ayuso
     [not found]   ` <014e01d1e21e$062884b0$12798e10$@ikuai8.com>
2016-07-20  8:50     ` Pablo Neira Ayuso [this message]
2016-07-20  8:50       ` 答复: " Pablo Neira Ayuso
2016-07-20  8:57       ` 答复: " 高峰
2016-07-20  8:57         ` 高峰
2016-07-20  0:51 ` Liping Zhang
2016-07-20  0:51   ` Liping Zhang
2016-07-20  1:02   ` 答复: " 高峰
2016-07-20  1:02     ` 高峰
2016-07-20  8:41     ` Pablo Neira Ayuso
2016-07-20  8:41       ` Pablo Neira Ayuso
2016-07-20  8:40   ` Pablo Neira Ayuso
2016-07-20  8:40     ` Pablo Neira Ayuso

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=20160720085018.GD1336@salvia \
    --to=pablo@netfilter.org \
    --cc=fgao@ikuai8.com.aqb.so \
    --cc=gfree.wind@gmail.com \
    --cc=kaber@trash.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.