threads / patch / 16872

patchAdd a commit.signoff configuration variable to always use --signoff.

Subject: [PATCH] Add a commit.signoff configuration variable to always use --signoff.

## tl;dr

19 messages between Dec 26, 2008 and Jan 2, 2009. Diffs are folded; open one to read it.

replies: 18people: 5as markdown or json

Adeodato Simó· Dec 26, 2008, 12:56 UTC · lore
Signed-off-by: Adeodato Simó <dato@net.com.org.es>
---
I wrote:
> Has there even been talk of a commit.signoff configuration variable
> to always add a S-o-b line? This could allow to enable it on a
> per-project basis, which would be cool.

Well, it seemed easy enough to do, so I went ahead. Comments would be welcome.

 Documentation/config.txt     |    6 ++++++
 Documentation/git-commit.txt |    3 ++-
 builtin-commit.c             |    5 +++++
 3 files changed, 13 insertions(+), 1 deletions(-)
Show changes to 3 files +13 −1

Documentation/config.txt, Documentation/git-commit.txt, builtin-commit.c

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 52786c7..6d195a3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -591,6 +591,12 @@ color.ui::
 commit.template::
 	Specify a file to use as the template for new commit messages.
 
+commit.signoff::
+	If set, 'git-commit' will always add a Signed-off-by line. If
+	you don't want it always active, you can still set it in the
+	repository specific configuration file for projects that require
+	a Signed-off-by line for all commits.
+
 diff.autorefreshindex::
 	When using 'git-diff' to compare with work tree
 	files, do not consider stat-only change as changed.
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index b5d81be..abab839 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -96,7 +96,8 @@ OPTIONS
 -s::
 --signoff::
 	Add Signed-off-by line by the committer at the end of the commit
-	log message.
+	log message. This overrides the `commit.signoff` configuration
+	variable.
 
 -n::
 --no-verify::
diff --git a/builtin-commit.c b/builtin-commit.c
index e88b78f..fc09539 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -929,6 +929,11 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	if (!strcmp(k, "commit.template"))
 		return git_config_string(&template_file, k, v);
 
+	if (!strcmp(k, "commit.signoff")) {
+		signoff = git_config_bool(k, v);
+		return 0;
+	}
+
 	return git_status_config(k, v, cb);
 }
 
-- 
1.6.1.307.g07803
Nanako Shiraishi· Dec 26, 2008, 22:02 UTC · re: Adeodato Simó · lore

Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff.

Quoting Adeodato Simó <dato@net.com.org.es>:
> I wrote:
>> Has there even been talk of a commit.signoff configuration variable
>> to always add a S-o-b line? This could allow to enable it on a
>> per-project basis, which would be cool.
There was a discussion in "http://thread.gmane.org/gmane.comp.version-control.git/32503/focus=32522" about automatically adding S-o-b line.
Even though Junio said in his response "it certainly is a possibility", another solution that he referred to as "cleaner and more useful" in his message was made available in version 1.5.3 after this discussion.
-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
Adeodato Simó· Dec 26, 2008, 22:10 UTC · re: Nanako Shiraishi · lore

Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff.

* Nanako Shiraishi [Sat, 27 Dec 2008 07:02:28 +0900]:
> There was a discussion in
> "http://thread.gmane.org/gmane.comp.version-control.git/32503/focus=32522"
> about automatically adding S-o-b line.
Ah, thanks for the pointer.
> Even though Junio said in his response "it certainly is a
> possibility", another solution that he referred to as "cleaner and
> more useful" in his message was made available in version 1.5.3 after
> this discussion.

Oh, a template file? I certainly hadn't thought of that, though I'd still like to hear if my patch would be suitable for inclusion, because it's much more straightforward to use (and to discover).

Thanks,
-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
- I love you, Shirley, I'm not ashamed to say.
- If you love me, then you'll want me to be happy. Even if I'm not with you.
- I don't love you that much.
                -- Denny Crane and Shirley Schmidt
Junio C Hamano· Dec 27, 2008, 00:36 UTC · re: Adeodato Simó · lore

Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff.

Adeodato Simó <dato@net.com.org.es> writes:
> ... I'd
> still like to hear if my patch would be suitable for inclusion, because
> it's much more straightforward to use (and to discover).
But "straightforward to discover" is not an advantage in this case.

As long as it comes with documentation that clearly explains why this feature should not be used blindly in order to avoid diluting the value of S-o-b, I think the feature itself is not a harmful thing to have. Dscho's argument in the quoted thread that says it should be a conscious act to add S-o-b (except for the part he misunderstands what S-o-b attests), is a good one and still is valid.

By the way, please do not deflect away responses meant to you by using a Mail-Followup-To header that points at the git mailing list. It is rude.

Adeodato Simó· Dec 27, 2008, 08:26 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff.

* Junio C Hamano [Fri, 26 Dec 2008 16:36:38 -0800]:
> Adeodato Simó <dato@net.com.org.es> writes:
> > ... I'd
> > still like to hear if my patch would be suitable for inclusion, because
> > it's much more straightforward to use (and to discover).
> But "straightforward to discover" is not an advantage in this case.
Show 6 quoted lines
> As long as it comes with documentation that clearly explains why this
> feature should not be used blindly in order to avoid diluting the value of
> S-o-b, I think the feature itself is not a harmful thing to have.  Dscho's
> argument in the quoted thread that says it should be a conscious act to
> add S-o-b (except for the part he misunderstands what S-o-b attests), is a
> good one and still is valid.

Does this sound good? If so, I'll send an amended patch (or should I send an incremental/extra one instead?):

+commit.signoff::
+       If set, 'git-commit' will always add a Signed-off-by line.
+       Please use this option with care: by enabling it, you're stating
+       that all your commits will invariably meet the S-o-b
+       requirements for any project you send patches to. It's probably
+       best to only use it from your private repositories' .git/config
+       file, and only for projects who require a S-o-b as proof of
+       provenance of the patch, and not of its correctness or quality.
> By the way, please do not deflect away responses meant to you by using a
> Mail-Followup-To header that points at the git mailing list.  It is rude.

I set a M-F-T header because I prefer not to be CC'ed. I have other mechanisms in place that prevent me from missing replies to my messages (based on In-Reply-To/References headers).

Nevertheless, if the list normally operates CC-based, I can see how pressing Reply-to-all and not seing the original autor in the recipient list can be offputting, so I'll stop setting M-F-T in my messages to git@. (Hopefully starting with this message already.)

I didn't mean for it to be rude in any way.
Thanks,
-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
A conference is a gathering of important people who singly can do nothing
but together can decide that nothing can be done.
                -- Fred Allen
Junio C Hamano· Dec 27, 2008, 08:44 UTC · re: Adeodato Simó · lore

Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff.

Adeodato Simó <dato@net.com.org.es> writes:
Show 5 quoted lines
> Does this sound good? If so, I'll send an amended patch (or should I send an
> incremental/extra one instead?):
>
> +commit.signoff::
> +       If set, 'git-commit' will always add a Signed-off-by line.

It is not a big deal, but my first reaction to the above was "S-o-b by whom? It misses details and does not say where to find them".

How about "if set, 'git commit' will behave as if '-s' option was given", so that we can leave the details of whose S-o-b line to generate and such to the description of "git commit -s" manual page?

Show 6 quoted lines
> +       Please use this option with care: by enabling it, you're stating
> +       that all your commits will invariably meet the S-o-b
> +       requirements for any project you send patches to. It's probably
> +       best to only use it from your private repositories' .git/config
> +       file, and only for projects who require a S-o-b as proof of
> +       provenance of the patch, and not of its correctness or quality.
Yeah, many projects do not even have S-o-b convention.
Show 11 quoted lines
>> By the way, please do not deflect away responses meant to you by using a
>> Mail-Followup-To header that points at the git mailing list.  It is rude.
>
> I set a M-F-T header because I prefer not to be CC'ed. I have other
> mechanisms in place that prevent me from missing replies to my messages
> (based on In-Reply-To/References headers).
>
> Nevertheless, if the list normally operates CC-based, I can see how pressing
> Reply-to-all and not seing the original autor in the recipient list can be
> offputting, so I'll stop setting M-F-T in my messages to git@. (Hopefully
> starting with this message already.)
Thanks.

Another problem that you seem to have missed about M-F-T is that while you solicited responses from general public by CC'ing the list (which allowed me to respond to you), by forcing the response go only to the list, you excluded people on the To: and Cc: list of your original message from my response. You required them to be subscribed to the list, if they want to be kept in the loop.

Adeodato Simó· Dec 27, 2008, 09:03 UTC · re: Junio C Hamano · lore

[PATCH] Add a commit.signoff configuration variable to always use --signoff

Signed-off-by: Adeodato Simó <dato@net.com.org.es>
---
* Junio C Hamano [Sat, 27 Dec 2008 00:44:59 -0800]:
> > +commit.signoff::
> > +       If set, 'git-commit' will always add a Signed-off-by line.
> It is not a big deal, but my first reaction to the above was "S-o-b by
> whom?  It misses details and does not say where to find them".
> How about "if set, 'git commit' will behave as if '-s' option was given",
> so that we can leave the details of whose S-o-b line to generate and such
> to the description of "git commit -s" manual page?
Changed.
> > I'll send an amended patch (or should I send an incremental/extra
> > one instead?)

I didn't get an answer to this, so I'm sending an amended one, hoping that's the correct thing to do in this land.

Show 6 quoted lines
> Another problem that you seem to have missed about M-F-T is that while you
> solicited responses from general public by CC'ing the list (which allowed
> me to respond to you), by forcing the response go only to the list, you
> excluded people on the To: and Cc: list of your original message from my
> response.  You required them to be subscribed to the list, if they want to
> be kept in the loop.

No, not really, because Mutt will add to the M-F-T header all addresses that appear on the To or Cc headers.

 Documentation/config.txt     |    9 +++++++++
 Documentation/git-commit.txt |    3 ++-
 builtin-commit.c             |    5 +++++
 3 files changed, 16 insertions(+), 1 deletions(-)
Show changes to 3 files +16 −1

Documentation/config.txt, Documentation/git-commit.txt, builtin-commit.c

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 52786c7..4d0a79b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -591,6 +591,15 @@ color.ui::
 commit.template::
 	Specify a file to use as the template for new commit messages.
 
+commit.signoff::
+	If set, 'git commit' will behave as if '-s' option was given.
+	Please use this option with care: by enabling it, you're stating
+	that all your commits will invariably meet the S-o-b
+	requirements for any project you send patches to. It's probably
+	best to only use it from your private repositories' .git/config
+	file, and only for projects who require a S-o-b as proof of
+	provenance of the patch, and not of its correctness or quality.
+
 diff.autorefreshindex::
 	When using 'git-diff' to compare with work tree
 	files, do not consider stat-only change as changed.
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index b5d81be..abab839 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -96,7 +96,8 @@ OPTIONS
 -s::
 --signoff::
 	Add Signed-off-by line by the committer at the end of the commit
-	log message.
+	log message. This overrides the `commit.signoff` configuration
+	variable.
 
 -n::
 --no-verify::
diff --git a/builtin-commit.c b/builtin-commit.c
index e88b78f..fc09539 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -929,6 +929,11 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	if (!strcmp(k, "commit.template"))
 		return git_config_string(&template_file, k, v);
 
+	if (!strcmp(k, "commit.signoff")) {
+		signoff = git_config_bool(k, v);
+		return 0;
+	}
+
 	return git_status_config(k, v, cb);
 }
 
-- 
1.6.1.307.g07803
Thomas Rast· Dec 27, 2008, 11:04 UTC · re: Adeodato Simó · lore

Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff

Adeodato Simó wrote:
Show 7 quoted lines
> +commit.signoff::
> +	If set, 'git commit' will behave as if '-s' option was given.
> +	Please use this option with care: by enabling it, you're stating
> +	that all your commits will invariably meet the S-o-b
> +	requirements for any project you send patches to. It's probably
> +	best to only use it from your private repositories' .git/config
> +	file, and only for projects who require a S-o-b as proof of
                           ^^^^^^^^^^^^

"projects which ..." or "projects that ...". "Who" can only stand for people, not objects.

> +	provenance of the patch, and not of its correctness or quality.
-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Adeodato Simó· Dec 27, 2008, 11:05 UTC · re: Thomas Rast · lore

Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff

* Thomas Rast [Sat, 27 Dec 2008 12:04:11 +0100]:
Show 9 quoted lines
> Adeodato Simó wrote:
> > +commit.signoff::
> > +	If set, 'git commit' will behave as if '-s' option was given.
> > +	Please use this option with care: by enabling it, you're stating
> > +	that all your commits will invariably meet the S-o-b
> > +	requirements for any project you send patches to. It's probably
> > +	best to only use it from your private repositories' .git/config
> > +	file, and only for projects who require a S-o-b as proof of
>                            ^^^^^^^^^^^^
> "projects which ..." or "projects that ...".  "Who" can only stand for
> people, not objects.
Good catch, thank you. Hopefully Junio can amend.
-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
                                   Listening to: Miguel Bosé - Puede que
Junio C Hamano· Dec 27, 2008, 11:53 UTC · re: Adeodato Simó · lore

Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff

Adeodato Simó <dato@net.com.org.es> writes:
Show 5 quoted lines
> +commit.signoff::
> +	If set, 'git commit' will behave as if '-s' option was given.
> +	Please use this option with care: by enabling it, you're stating
> +	that all your commits will invariably meet the S-o-b
> +	requirements for any project you send patches to. It's probably

This is true only if you set it in your global configuration. A more sane usage would be to have it per repository as you recommend in a later sentence, and "for any project" part is untrue when the reader does so.

Show 11 quoted lines
> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> index b5d81be..abab839 100644
> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -96,7 +96,8 @@ OPTIONS
>  -s::
>  --signoff::
>  	Add Signed-off-by line by the committer at the end of the commit
> -	log message.
> +	log message. This overrides the `commit.signoff` configuration
> +	variable.
Good and careful thinking.  I like it.
Adeodato Simó· Dec 27, 2008, 12:01 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff

* Junio C Hamano [Sat, 27 Dec 2008 03:53:47 -0800]:
> Adeodato Simó <dato@net.com.org.es> writes:
Show 5 quoted lines
> > +commit.signoff::
> > +	If set, 'git commit' will behave as if '-s' option was given.
> > +	Please use this option with care: by enabling it, you're stating
> > +	that all your commits will invariably meet the S-o-b
> > +	requirements for any project you send patches to. It's probably
> This is true only if you set it in your global configuration.  A more sane
> usage would be to have it per repository as you recommend in a later
> sentence, and "for any project" part is untrue when the reader does so.
How about:
  Please use this option with care: by enabling it globally, you'd be
  stating...
-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
                              Listening to: Niza - Solsticio de invierno
Adeodato Simó· Dec 29, 2008, 11:16 UTC · re: Adeodato Simó · lore

[PATCH v3] Add a commit.signoff configuration option to always use --signoff in commit

The intent is that it only applies when the user runs `git commit` themselves, hence a number of commands (rebase and revert/cherry-pick) have started passing --no-signoff when invoking commit.

Signed-off-by: Adeodato Simó <dato@net.com.org.es>
---

Hello, I've worked a bit more on this patch, and I'd like to know if it's going in a good direction or not. Changes since v2:

 t/t7500-commit.sh        |   22 ++++++++++++++++++++++
   * add tests to check that commit.signoff works correctly, and that 
     is overriden by --no-signoff
 git-rebase.sh            |    2 +-
 t/t3402-rebase-merge.sh  |    6 +++++-
   * make git-rebase pass --no-signoff when invoking `git commit`, and
     add a test for it
   
 builtin-revert.c         |    2 ++
   * make revert/cherry-pick pass --no-signoff unless -s was given by
     the user (missing test)
   
 Documentation/config.txt |    4 ++--
   * improve config.txt wording as per <20081227120128.GA11322@chistera.yi.org>

(I don't know if it's customary to send a diff from v2 to v3, if it is please let me know.)

Thanks,
 Documentation/config.txt     |    9 +++++++++
 Documentation/git-commit.txt |    3 ++-
 builtin-commit.c             |    5 +++++
 builtin-revert.c             |    2 ++
 git-rebase.sh                |    2 +-
 t/t3402-rebase-merge.sh      |    6 +++++-
 t/t7500-commit.sh            |   22 ++++++++++++++++++++++
 7 files changed, 46 insertions(+), 3 deletions(-)
Show changes to 7 files +46 −3

Documentation/config.txt, Documentation/git-commit.txt, builtin-commit.c, builtin-revert.c, git-rebase.sh, t/t3402-rebase-merge.sh, t/t7500-commit.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 52786c7..13f2200 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -591,6 +591,15 @@ color.ui::
 commit.template::
 	Specify a file to use as the template for new commit messages.
 
+commit.signoff::
+	If set, 'git commit' will behave as if '-s' option was given.
+	Please use this option with care: by enabling it globally, you'd
+	be stating that all your commits will invariably meet the S-o-b
+	requirements for any project you send patches to. It's probably
+	best to only use it from your private repositories' .git/config
+	file, and only for projects who require a S-o-b as proof of
+	provenance of the patch, and not of its correctness or quality.
+
 diff.autorefreshindex::
 	When using 'git-diff' to compare with work tree
 	files, do not consider stat-only change as changed.
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index b5d81be..abab839 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -96,7 +96,8 @@ OPTIONS
 -s::
 --signoff::
 	Add Signed-off-by line by the committer at the end of the commit
-	log message.
+	log message. This overrides the `commit.signoff` configuration
+	variable.
 
 -n::
 --no-verify::
diff --git a/builtin-commit.c b/builtin-commit.c
index e88b78f..fc09539 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -929,6 +929,11 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	if (!strcmp(k, "commit.template"))
 		return git_config_string(&template_file, k, v);
 
+	if (!strcmp(k, "commit.signoff")) {
+		signoff = git_config_bool(k, v);
+		return 0;
+	}
+
 	return git_status_config(k, v, cb);
 }
 
diff --git a/builtin-revert.c b/builtin-revert.c
index d48313c..395c7a5 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -429,6 +429,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 		args[i++] = "-n";
 		if (signoff)
 			args[i++] = "-s";
+		else
+			args[i++] = "--no-signoff";
 		if (!edit) {
 			args[i++] = "-F";
 			args[i++] = defmsg;
diff --git a/git-rebase.sh b/git-rebase.sh
index ebd4df3..bf520d0 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -63,7 +63,7 @@ continue_merge () {
 	cmt=`cat "$dotest/current"`
 	if ! git diff-index --quiet --ignore-submodules HEAD --
 	then
-		if ! git commit --no-verify -C "$cmt"
+		if ! git commit --no-verify --no-signoff -C "$cmt"
 		then
 			echo "Commit failed, please do not call \"git commit\""
 			echo "directly, but instead do one of the following: "
diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh
index 7b7d072..bd2d08c 100755
--- a/t/t3402-rebase-merge.sh
+++ b/t/t3402-rebase-merge.sh
@@ -41,7 +41,8 @@ test_expect_success setup '
 	git branch test-rebase side &&
 	git branch test-rebase-pick side &&
 	git branch test-reference-pick side &&
-	git checkout -b test-merge side
+	git checkout -b test-merge side &&
+	git config commit.signoff true
 '
 
 test_expect_success 'reference merge' '
@@ -54,6 +55,9 @@ test_expect_success rebase '
 	GIT_TRACE=1 git rebase --merge master
 '
 
+test_expect_success 'rebase uses --no-signoff' '
+	!(git log | grep -q Signed-off-by)'
+
 test_expect_success 'test-rebase@{1} is pre rebase' '
 	test $PRE_REBASE = $(git rev-parse test-rebase@{1})
 '
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index 6e18a96..c7deb3e 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -147,6 +147,10 @@ zort
 Signed-off-by: C O Mitter <committer@example.com>
 EOF
 
+cat > expect_no_signoff << EOF
+zort
+EOF
+
 test_expect_success '--signoff' '
 	echo "yet another content *narf*" >> foo &&
 	echo "zort" | (
@@ -157,6 +161,24 @@ test_expect_success '--signoff' '
 	test_cmp expect output
 '
 
+test_expect_success 'commit.signoff' '
+	echo "and more content" >> foo &&
+	git config commit.signoff true &&
+	echo "zort" | git commit -F - foo &&
+	git config --unset commit.signoff &&
+	git cat-file commit HEAD | sed "1,/^$/d" > output &&
+	test_cmp expect output
+'
+
+test_expect_success '--no-signoff trumps commit.signoff' '
+	echo "and even more" >> foo &&
+	git config commit.signoff true &&
+	echo "zort" | git commit --no-signoff -F - foo &&
+	git config --unset commit.signoff &&
+	git cat-file commit HEAD | sed "1,/^$/d" > output &&
+	test_cmp expect_no_signoff output
+'
+
 test_expect_success 'commit message from file (1)' '
 	mkdir subdir &&
 	echo "Log in top directory" >log &&
-- 
1.6.1.307.g07803
Adeodato Simó· Dec 29, 2008, 11:18 UTC · re: Adeodato Simó · lore

Re: [PATCH v3] Add a commit.signoff configuration option to always use --signoff in commit

>  Changes since v2:
(Updating git-rebase--interactive.sh is still missing, I know.)
-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
He who has not a good memory should never take upon himself the trade of lying.
                -- Michel de Montaigne
Junio C Hamano· Dec 30, 2008, 21:04 UTC · re: Adeodato Simó · lore

Re: [PATCH v3] Add a commit.signoff configuration option to always use --signoff in commit

Adeodato Simó <dato@net.com.org.es> writes:
Show 13 quoted lines
> diff --git a/builtin-revert.c b/builtin-revert.c
> index d48313c..395c7a5 100644
> --- a/builtin-revert.c
> +++ b/builtin-revert.c
> @@ -429,6 +429,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)
>  		args[i++] = "-n";
>  		if (signoff)
>  			args[i++] = "-s";
> +		else
> +			args[i++] = "--no-signoff";
>  		if (!edit) {
>  			args[i++] = "-F";
>  			args[i++] = defmsg;

Introduction of --no-commit to "git commit" is absolutely necessary for interactive users if we were to introduce commit.signoff, but I am not sure about this change and similar ones given to the other Porcelain commands that use "git commit". They actually started making me think that commmit.signoff might be more trouble than it is worth.

It is plausible that your workflow is to have others push to your 'mob' branch and integrating good bits from there by cherry-picking, sort of like using the 'mob' branch as if they are e-mailed patches. In such a setup, if you are so forgetful to type "-s" for your commit that you would want commit.signoff configuration, you would likely to be equally forgetful to type "-s" for your cherry-pick, and would want to have some configuration affect how this part of the code works.

I however moderately doubt if the complexity and flexibility of having rebase.signoff, revert.signoff, and cherry-pick.signoff as independent options is worth it. I am inclined to think that is too many knobs to tweak, and it is far simpler to understand and easier to explain if the single configuration, commit.signoff, applied to every Porcelain that creates commits.

If we were to go that route, instead of passing --no-signoff when they invoke "git commit", these commands need to take their own --no-signoff option instead, and when neither --signoff nor --no-signoff is given, they just should just invoke "git commit" and let it use the config (if set).

Adeodato Simó· Jan 1, 2009, 22:18 UTC · re: Junio C Hamano · lore

Re: [PATCH v3] Add a commit.signoff configuration option to always use --signoff in commit

* Junio C Hamano [Tue, 30 Dec 2008 13:04:13 -0800]:
> They actually started making me think
> that commmit.signoff might be more trouble than it is worth.

I am beginning to think the same myself, and I'm okay with letting go. If somebody has a sensible plan, I can invest some time on implementing it, but I'll reckon it all tastes too messy at the moment.

Thanks for your time.
-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
                                     Listening to: Vanessa-Mae - Leyenda
Adeodato Simó· Jan 2, 2009, 12:46 UTC · re: Adeodato Simó · lore

Re: [PATCH v3] Add a commit.signoff configuration option to always use --signoff in commit

* Adeodato Simó [Thu, 01 Jan 2009 23:18:17 +0100]:
> * Junio C Hamano [Tue, 30 Dec 2008 13:04:13 -0800]:
> > They actually started making me think
> > that commmit.signoff might be more trouble than it is worth.
> I am beginning to think the same myself, and I'm okay with letting go.
> If somebody has a sensible plan, I can invest some time on implementing
> it, but I'll reckon it all tastes too messy at the moment.
> Thanks for your time.

Oh, I see you merged the last version into pu. I don't know if that means it'd be bad for me to abandon now, but as said, if somebody helps me figure out things, I'm open to continue working on it.

I just don't want to be pushing for inclusion of something of sub-standard (UI) quality.

Cheers,
-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
Will you just stand still?
                -- Luke Danes
Jan Krüger· Dec 27, 2008, 18:08 UTC · re: Adeodato Simó · lore

Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff.

Hi,

On Fri, 26 Dec 2008 13:56:59 +0100 "Adeodato Simó" <dato@net.com.org.es> wrote:

Show 6 quoted lines
> > Has there even been talk of a commit.signoff configuration variable
> > to always add a S-o-b line? This could allow to enable it on a
> > per-project basis, which would be cool.
> 
> Well, it seemed easy enough to do, so I went ahead. Comments would be
> welcome.

I think it might be a good idea to allow overriding the config variable in the other direction, i.e. a --no-signoff option to commit. Otherwise, for example, rebase would have no way of suppressing the S-o-b lines in rebased commits (and you might want to not automatically sign off commits you rebase).

-Jan
Adeodato Simó· Dec 27, 2008, 18:40 UTC · re: Jan Krüger · lore

Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff.

* Jan Krüger [Sat, 27 Dec 2008 19:08:19 +0100]:
Show 5 quoted lines
> I think it might be a good idea to allow overriding the config variable
> in the other direction, i.e. a --no-signoff option to commit. Otherwise,
> for example, rebase would have no way of suppressing the S-o-b lines in
> rebased commits (and you might want to not automatically sign off
> commits you rebase).
Good catch.
--no-signoff exists already, so maybe git-rebase should just use it?
-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
Man is certainly stark mad; he cannot make a flea, yet he makes gods by the
dozens.
                -- Michel de Montaigne
Jan Krüger· Dec 27, 2008, 19:15 UTC · re: Adeodato Simó · lore

Re: [PATCH] Add a commit.signoff configuration variable to always use --signoff.

Hi,

On Sat, 27 Dec 2008 19:40:01 +0100 "Adeodato Simó" <dato@net.com.org.es> wrote:

Show 7 quoted lines
> > I think it might be a good idea to allow overriding the config
> > variable in the other direction, i.e. a --no-signoff option to
> > commit. [...]
> 
> Good catch.
> 
> --no-signoff exists already, so maybe git-rebase should just use it?

Oh, yeah, I didn't notice the option mechanism worked this way and the options got parsed after processing the config variables. I guess rebase and rebase--interactive should use it, then, and perhaps there are other commands that should that I just don't know about (a grep across all commands implemented in shell script didn't find anything else, though).

-Jan

← back to recent threads