Re: [PATCH v2] Place cache.h at the first place to match general rule
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
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
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