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

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

[Adeodato Simó]

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(-)

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

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

[Nanako Shiraishi]

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/

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

[Adeodato Simó]

* 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

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

[Junio C Hamano]

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.

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

[Adeodato Simó]

* 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.

> 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

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

[Junio C Hamano]

Adeodato Simó <dato@net.com.org.es> writes:

> 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?

> +       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.

>> 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.

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

[Adeodato Simó]

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.

> 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(-)

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

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

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 709 bytes --]

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.

> +	provenance of the patch, and not of its correctness or quality.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch






[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

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

[Adeodato Simó]

* Thomas Rast [Sat, 27 Dec 2008 12:04:11 +0100]:

> 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

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

[Junio C Hamano]

Adeodato Simó <dato@net.com.org.es> writes:

> +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.

> 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.

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

[Adeodato Simó]

* Junio C Hamano [Sat, 27 Dec 2008 03:53:47 -0800]:

> Adeodato Simó <dato@net.com.org.es> writes:

> > +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

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

[Jan Krüger]

Hi,

On Fri, 26 Dec 2008 13:56:59 +0100
"Adeodato Simó" <dato@net.com.org.es> 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.

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

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

[Adeodato Simó]

* Jan Krüger [Sat, 27 Dec 2008 19:08:19 +0100]:

> 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

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

[Jan Krüger]

Hi,

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

> > 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

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

[Adeodato Simó]

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(-)

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

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

[Adeodato Simó]

>  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

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

[Junio C Hamano]

Adeodato Simó <dato@net.com.org.es> writes:

> 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).

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

[Adeodato Simó]

* 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

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

[Adeodato Simó]

* 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