Re: [PATCH v2] Place cache.h at the first place to match general rule

2014-03-02 Thread He Sun
2014-03-02 12:34 GMT+08:00 Eric Sunshine sunsh...@sunshineco.com:
 On Sat, Mar 1, 2014 at 9:18 PM, Sun He sunheeh...@gmail.com wrote:
 Signed-off-by: Sun He sunheeh...@gmail.com
 Helped-by: Duy Nguyen pclo...@gmail.com

 Footers should follow a temporal order. For instance:

 1. Duy helped you.
 2. You revised your patch based upon his input.
 3. You signed off before submitting the patch.

 Hence, your Signed-off-by: should follow Helped-by:.


OK, got it.
Thank you very much.

 ---
  PATCH v2 Fix the spelling bug of general in subject as is suggested
  by brain m.calson sand...@crustytoothpaste.net

 There are two type of information you want to convey to readers:

 1. Explanation and justification of the change itself. This is
 recorded for all time in the project history as the commit message. It
 is placed above the --- line.

 2. Commentary related to this version / submission of the patch which
 is not likely to be helpful or meaningful to people reading the
 official project history via the commit messages. It is placed below
 the --- line.

 Explaining what you changed since the previous version of the patch,
 as you do above, is a good thing. It's not meaningful once the patch
 is officially accepted into the project; it's only meaningful to
 people following the progression of the patch on the mailing list, so
 it definitely belongs below the --- line, as you did here.

 However...

  The general rule is if cache.h or git-compat-util.h is included,
  it is the first #include.

 This information explains the patch's purpose, thus it is relevant to
 the project history. It belongs above the --- line.


OK, Got it. I will move above the --- line soon, and I will pay attention
not to make mistakes like this from now on.
Thank you very much.

  I parsed all the source files, and find many files start with builtin.h.
  And git-compat-util.h is the first in it. So they don't need any change.

 This could go either way. It tells how you arrived at this version of
 the patch (relevant below ---), but also explains why the patch does
 not have to touch additional files (relevant above ---). It's
 probably okay to leave it below ---.

  sigchain.c and test-sigchain.c are started with sigchain.h
  I checked sigchain.h, and it didn't import any bug.
  But to keep consistant with general rule, we should take this patch.

 Commentary suitable for below ---.

  Thanks.

  sigchain.c  | 2 +-
  test-sigchain.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

 diff --git a/sigchain.c b/sigchain.c
 index 1118b99..faa375d 100644
 --- a/sigchain.c
 +++ b/sigchain.c
 @@ -1,5 +1,5 @@
 -#include sigchain.h
  #include cache.h
 +#include sigchain.h

  #define SIGCHAIN_MAX_SIGNALS 32

 diff --git a/test-sigchain.c b/test-sigchain.c
 index 42db234..e499fce 100644
 --- a/test-sigchain.c
 +++ b/test-sigchain.c
 @@ -1,5 +1,5 @@
 -#include sigchain.h
  #include cache.h
 +#include sigchain.h

  #define X(f) \
  static void f(int sig) { \
 --
 1.9.0.138.g2de3478.dirty


I am honored to get detailed guidance from you.
Thank you very very much.

He Sun
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] Place cache.h at the first place to match general rule

2014-03-01 Thread Sun He
Signed-off-by: Sun He sunheeh...@gmail.com
Helped-by: Duy Nguyen pclo...@gmail.com
---

 PATCH v2 Fix the spelling bug of general in subject as is suggested
 by brain m.calson sand...@crustytoothpaste.net

 The general rule is if cache.h or git-compat-util.h is included,
 it is the first #include.

 I parsed all the source files, and find many files start with builtin.h.
 And git-compat-util.h is the first in it. So they don't need any change.

 sigchain.c and test-sigchain.c are started with sigchain.h
 I checked sigchain.h, and it didn't import any bug.
 But to keep consistant with general rule, we should take this patch.

 Thanks.

 sigchain.c  | 2 +-
 test-sigchain.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sigchain.c b/sigchain.c
index 1118b99..faa375d 100644
--- a/sigchain.c
+++ b/sigchain.c
@@ -1,5 +1,5 @@
-#include sigchain.h
 #include cache.h
+#include sigchain.h
 
 #define SIGCHAIN_MAX_SIGNALS 32
 
diff --git a/test-sigchain.c b/test-sigchain.c
index 42db234..e499fce 100644
--- a/test-sigchain.c
+++ b/test-sigchain.c
@@ -1,5 +1,5 @@
-#include sigchain.h
 #include cache.h
+#include sigchain.h
 
 #define X(f) \
 static void f(int sig) { \
-- 
1.9.0.138.g2de3478.dirty

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


Re: [PATCH v2] Place cache.h at the first place to match general rule

2014-03-01 Thread Eric Sunshine
On Sat, Mar 1, 2014 at 9:18 PM, Sun He sunheeh...@gmail.com wrote:
 Signed-off-by: Sun He sunheeh...@gmail.com
 Helped-by: Duy Nguyen pclo...@gmail.com

Footers should follow a temporal order. For instance:

1. Duy helped you.
2. You revised your patch based upon his input.
3. You signed off before submitting the patch.

Hence, your Signed-off-by: should follow Helped-by:.

 ---
  PATCH v2 Fix the spelling bug of general in subject as is suggested
  by brain m.calson sand...@crustytoothpaste.net

There are two type of information you want to convey to readers:

1. Explanation and justification of the change itself. This is
recorded for all time in the project history as the commit message. It
is placed above the --- line.

2. Commentary related to this version / submission of the patch which
is not likely to be helpful or meaningful to people reading the
official project history via the commit messages. It is placed below
the --- line.

Explaining what you changed since the previous version of the patch,
as you do above, is a good thing. It's not meaningful once the patch
is officially accepted into the project; it's only meaningful to
people following the progression of the patch on the mailing list, so
it definitely belongs below the --- line, as you did here.

However...

  The general rule is if cache.h or git-compat-util.h is included,
  it is the first #include.

This information explains the patch's purpose, thus it is relevant to
the project history. It belongs above the --- line.

  I parsed all the source files, and find many files start with builtin.h.
  And git-compat-util.h is the first in it. So they don't need any change.

This could go either way. It tells how you arrived at this version of
the patch (relevant below ---), but also explains why the patch does
not have to touch additional files (relevant above ---). It's
probably okay to leave it below ---.

  sigchain.c and test-sigchain.c are started with sigchain.h
  I checked sigchain.h, and it didn't import any bug.
  But to keep consistant with general rule, we should take this patch.

Commentary suitable for below ---.

  Thanks.

  sigchain.c  | 2 +-
  test-sigchain.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

 diff --git a/sigchain.c b/sigchain.c
 index 1118b99..faa375d 100644
 --- a/sigchain.c
 +++ b/sigchain.c
 @@ -1,5 +1,5 @@
 -#include sigchain.h
  #include cache.h
 +#include sigchain.h

  #define SIGCHAIN_MAX_SIGNALS 32

 diff --git a/test-sigchain.c b/test-sigchain.c
 index 42db234..e499fce 100644
 --- a/test-sigchain.c
 +++ b/test-sigchain.c
 @@ -1,5 +1,5 @@
 -#include sigchain.h
  #include cache.h
 +#include sigchain.h

  #define X(f) \
  static void f(int sig) { \
 --
 1.9.0.138.g2de3478.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html