Re: [PATCH v2] commit: add commit.signoff config option

2015-06-25 Thread Caio Marcelo de Oliveira Filho
Hi,

> Hmm, I do not recall seeing v1 but that is OK.

http://thread.gmane.org/gmane.comp.version-control.git/272635
http://thread.gmane.org/gmane.comp.version-control.git/272636


> The following immediately comes to mind.
>
> http://thread.gmane.org/gmane.comp.version-control.git/51754/focus=51780

Thanks for the reference, amazed that I ended up using the same title!
>From the thread:

> Even though these lines are not digitally signed,
> the intent of adding a Signed-off-by: line with your name is
> that you are certifying its origin, according to the definition
> of DCO (see Documentation/SubmittingPatches).  This should be a
> conscious act from the signer's part, and making it automatic
> with a config variable that you set once and forget makes it
> much less meaningful.

This is a fair point. However I've seen that in practice, in some
cases it's easier
to consider the DCO as the rules for contributing to the entire
project. In those
cases people tend to use commit templates or aliases or hooks, which in practice
automate the signing off act anyway.

A similar concern probably applies to format.signOff option. Would be sufficient
to add a note about conscious act (like format.signOff has) to the
config description?


-- 
Caio Marcelo de Oliveira Filho
--
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] commit: add commit.signoff config option

2015-06-25 Thread Caio Marcelo de Oliveira Filho
On Thu, Jun 25, 2015 at 5:43 AM, Christian Couder
 wrote:
> You can use a commit template.
>
> Or you can use the "commit-msg" hook. For example with commands like:
>
> grep "^Signed-off-by:" "$1" ||
> echo "Signed-off-by: $(git config user.name) <$(git config user.email)>" 
> >>"$1"
>
> If you have more complex needs, there is also "git interpret-trailers"
> (see the examples in the man page).

Thanks for the references. I was initially using a commit template to solve
the problem. But I've considered that since we already handle
"Signed-off-by" somewhat specially (e.g. "commit -s"), and how common this
case is, it was worth adding a config option.


-- 
Caio Marcelo de Oliveira Filho
--
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] commit: add commit.signoff config option

2015-06-25 Thread Caio Marcelo de Oliveira Filho
> In both tests, you should probably use 'test_config' instead of 'git
> config [...] git config --unset', it takes care of it for you (also
> should prevent the case where the config is not unset when your test
> fails in the middle).

Posted a v2 with this improvement and the bash completion.
Thanks for the review.


-- 
Caio Marcelo de Oliveira Filho
--
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] commit: add commit.signoff config option

2015-06-25 Thread Caio Marcelo de Oliveira Filho
In projects that use Signed-off-by, it's convenient to include that line
in the commit message by default. The commit.signoff config option
allows to add that line in all commits automatically.

Document that this config option can be overriden by using
--no-signoff.

Signed-off-by: Caio Marcelo de Oliveira Filho 
---

Differences from v1:
* Use test_config helper in the test
* Add bash completion for commit and config

 Documentation/config.txt   |  6 ++
 Documentation/git-commit.txt   |  5 +
 builtin/commit.c   |  4 
 contrib/completion/git-completion.bash |  4 +++-
 t/t7500-commit.sh  | 20 
 5 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3e37b93..e019f62 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1089,6 +1089,12 @@ commit.gpgSign::
convenient to use an agent to avoid typing your GPG passphrase
several times.
 
+commit.signoff::
+
+   A boolean to enable/disable whether Signed-off-by line by the
+   committer should be added to all commits at the end of the
+   commit log messages.  Defaults to false.
+
 commit.status::
A boolean to enable/disable inclusion of status information in the
commit message template when using an editor to prepare the commit
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 904dafa..7546c7a 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -156,6 +156,11 @@ OPTIONS
Add Signed-off-by line by the committer at the end of the commit
log message.
 
+--no-signoff::
+   Countermand `commit.signoff` configuration, preventing a
+   Signed-off-by line to be added at the end of the commit log
+   message.
+
 -n::
 --no-verify::
This option bypasses the pre-commit and commit-msg hooks.
diff --git a/builtin/commit.c b/builtin/commit.c
index 254477f..5cfbe57 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1505,6 +1505,10 @@ static int git_commit_config(const char *k, const char 
*v, void *cb)
sign_commit = git_config_bool(k, v) ? "" : NULL;
return 0;
}
+   if (!strcmp(k, "commit.signoff")) {
+   signoff = git_config_bool(k, v);
+   return 0;
+   }
 
status = git_gpg_config(k, v, NULL);
if (status)
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index c97c648..7a79a89 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1123,7 +1123,8 @@ _git_commit ()
;;
--*)
__gitcomp "
-   --all --author= --signoff --verify --no-verify
+   --all --author=
+   --signoff --no-signoff --verify --no-verify
--edit --no-edit
--amend --include --only --interactive
--dry-run --reuse-message= --reedit-message=
@@ -2009,6 +2010,7 @@ _git_config ()
color.status.untracked
color.status.updated
color.ui
+   commit.signoff
commit.status
commit.template
core.abbrev
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index 116885a..949272d 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -179,6 +179,26 @@ test_expect_success '--signoff' '
test_cmp expect output
 '
 
+test_expect_success 'commit.signoff config option' '
+   test_config commit.signoff true &&
+   echo "yet another content *narf*" >> foo &&
+   echo "zort" | git commit -F - foo &&
+   git cat-file commit HEAD | sed "1,/^\$/d" > output &&
+   test_cmp expect output
+'
+
+cat > expect <> foo &&
+   echo "no signed off by here" | git commit --no-signoff -F - foo &&
+   git cat-file commit HEAD | sed "1,/^\$/d" > output &&
+   test_cmp expect output
+'
+
 test_expect_success 'commit message from file (1)' '
mkdir subdir &&
echo "Log in top directory" >log &&
-- 
2.4.4.489.g5bc41de

--
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] commit: add commit.signoff config option

2015-06-25 Thread Caio Marcelo de Oliveira Filho
In projects that use Signed-off-by, it's convenient to include that line
in the commit by default. The commit.signoff config option allows to add
that line in all commits automatically.

Document that this config option can be overriden by using
--no-signoff.

Signed-off-by: Caio Marcelo de Oliveira Filho 
---
 Documentation/config.txt |  6 ++
 Documentation/git-commit.txt |  5 +
 builtin/commit.c |  4 
 t/t7500-commit.sh| 22 ++
 4 files changed, 37 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3e37b93..e019f62 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1089,6 +1089,12 @@ commit.gpgSign::
convenient to use an agent to avoid typing your GPG passphrase
several times.
 
+commit.signoff::
+
+   A boolean to enable/disable whether Signed-off-by line by the
+   committer should be added to all commits at the end of the
+   commit log messages.  Defaults to false.
+
 commit.status::
A boolean to enable/disable inclusion of status information in the
commit message template when using an editor to prepare the commit
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 904dafa..7546c7a 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -156,6 +156,11 @@ OPTIONS
Add Signed-off-by line by the committer at the end of the commit
log message.
 
+--no-signoff::
+   Countermand `commit.signoff` configuration, preventing a
+   Signed-off-by line to be added at the end of the commit log
+   message.
+
 -n::
 --no-verify::
This option bypasses the pre-commit and commit-msg hooks.
diff --git a/builtin/commit.c b/builtin/commit.c
index 254477f..5cfbe57 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1505,6 +1505,10 @@ static int git_commit_config(const char *k, const char 
*v, void *cb)
sign_commit = git_config_bool(k, v) ? "" : NULL;
return 0;
}
+   if (!strcmp(k, "commit.signoff")) {
+   signoff = git_config_bool(k, v);
+   return 0;
+   }
 
status = git_gpg_config(k, v, NULL);
if (status)
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index 116885a..fcb39b4 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -179,6 +179,28 @@ test_expect_success '--signoff' '
test_cmp expect output
 '
 
+test_expect_success 'commit.signoff config option' '
+   git config commit.signoff true &&
+   echo "yet another content *narf*" >> foo &&
+   echo "zort" | git commit -F - foo &&
+   git cat-file commit HEAD | sed "1,/^\$/d" > output &&
+   git config --unset commit.signoff &&
+   test_cmp expect output
+'
+
+cat > expect <> foo &&
+   echo "no signed off by here" | git commit --no-signoff -F - foo &&
+   git cat-file commit HEAD | sed "1,/^\$/d" > output &&
+   git config --unset commit.signoff &&
+   test_cmp expect output
+'
+
 test_expect_success 'commit message from file (1)' '
mkdir subdir &&
echo "Log in top directory" >log &&
-- 
2.4.4.489.gc2a2b54.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