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

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

[Sun He]

Signed-off-by: Sun He <sunheehnus@gmail.com>
Helped-by: Duy Nguyen <pclouds@gmail.com>
---

 "PATCH v2" Fix the spelling bug of general in subject as is suggested
 by brain m.calson <sandals@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

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

[Eric Sunshine]

On Sat, Mar 1, 2014 at 9:18 PM, Sun He <sunheehnus@gmail.com> wrote:
> Signed-off-by: Sun He <sunheehnus@gmail.com>
> Helped-by: Duy Nguyen <pclouds@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 <sandals@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

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

[He Sun]

2014-03-02 12:34 GMT+08:00 Eric Sunshine <sunshine@sunshineco.com>:
> On Sat, Mar 1, 2014 at 9:18 PM, Sun He <sunheehnus@gmail.com> wrote:
>> Signed-off-by: Sun He <sunheehnus@gmail.com>
>> Helped-by: Duy Nguyen <pclouds@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 <sandals@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