git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cg-commit: Fix a typo that would inhibit running of post-commit script:
@ 2006-08-24 18:19 Jim Meyering
  2006-08-25  0:20 ` Jonas Fonseca
  0 siblings, 1 reply; 4+ messages in thread
From: Jim Meyering @ 2006-08-24 18:19 UTC (permalink / raw)
  To: git

I tried cg-commit with a commit hook, but the hook never ran.
The problem was a typo:

Fix a typo that would inhibit running the post-commit script:
s/commit-post/post-commit/.

Signed-off-by: Jim Meyering <jim@meyering.net>
---
 cg-commit |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/cg-commit b/cg-commit
index 9d3b1a1..82eea60 100755
--- a/cg-commit
+++ b/cg-commit
@@ -604,15 +604,16 @@ if [ "$newhead" ]; then
 		branchname="$(cat "$_git/branch-name")"
 	fi
 	[ -z "$branchname" ] && [ "$_git_head" != "master" ] && branchname="$_git_head"
-	if [ -x "$_git/hooks/post-commit" -a ! "$no_hooks" ]; then
+	post_commit="$_git/hooks/post-commit"
+	if [ -x "$post_commit" -a ! "$no_hooks" ]; then
 		if [ "$(git-repo-config --bool cogito.hooks.commit.post.allmerged)" = "true" ]; then
 			# We just hope that for the initial commit, the user didn't
 			# manage to install the hook yet.
 			for merged in $(git-rev-list $newhead ^$oldhead | tac); do
-				"$_git/hooks/post-commit" "$merged" "$branchname"
+				"$post_commit" "$merged" "$branchname"
 			done
 		else
-			"$_git/hooks/post-commit" "$newhead" "$branchname"
+			"$post_commit" "$newhead" "$branchname"
 		fi
 	fi
 
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] cg-commit: Fix a typo that would inhibit running of post-commit script:
  2006-08-24 18:19 [PATCH] cg-commit: Fix a typo that would inhibit running of post-commit script: Jim Meyering
@ 2006-08-25  0:20 ` Jonas Fonseca
  2006-09-16 15:10   ` Jim Meyering
  0 siblings, 1 reply; 4+ messages in thread
From: Jonas Fonseca @ 2006-08-25  0:20 UTC (permalink / raw)
  To: Jim Meyering; +Cc: git

Jim Meyering <jim@meyering.net> wrote Thu, Aug 24, 2006:
> I tried cg-commit with a commit hook, but the hook never ran.
> The problem was a typo:
> 
> Fix a typo that would inhibit running the post-commit script:
> s/commit-post/post-commit/.

If I remember correctly, historically, the commit-post existed before
the post-commit appeared. You can see that it is documented in the man
page so it is not a typo.

However, this should certainly be updated, but I think a better fix
would be to transitionally warn the user about the existence of the
commit-post hook before using it in favour of post-commit.

> diff --git a/cg-commit b/cg-commit
> index 9d3b1a1..82eea60 100755
> --- a/cg-commit
> +++ b/cg-commit
> @@ -604,15 +604,16 @@ if [ "$newhead" ]; then
>  		branchname="$(cat "$_git/branch-name")"
>  	fi
>  	[ -z "$branchname" ] && [ "$_git_head" != "master" ] && branchname="$_git_head"
> -	if [ -x "$_git/hooks/post-commit" -a ! "$no_hooks" ]; then
> +	post_commit="$_git/hooks/post-commit"
> +	if [ -x "$post_commit" -a ! "$no_hooks" ]; then
>  		if [ "$(git-repo-config --bool cogito.hooks.commit.post.allmerged)" = "true" ]; then
>  			# We just hope that for the initial commit, the user didn't
>  			# manage to install the hook yet.
>  			for merged in $(git-rev-list $newhead ^$oldhead | tac); do
> -				"$_git/hooks/post-commit" "$merged" "$branchname"
> +				"$post_commit" "$merged" "$branchname"
>  			done
>  		else
> -			"$_git/hooks/post-commit" "$newhead" "$branchname"
> +			"$post_commit" "$newhead" "$branchname"
>  		fi
>  	fi

The patch looks more like a refactoring of a previous commit that did
the commit-post -> post-commit replacement.

-- 
Jonas Fonseca

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cg-commit: Fix a typo that would inhibit running of post-commit script:
  2006-08-25  0:20 ` Jonas Fonseca
@ 2006-09-16 15:10   ` Jim Meyering
  2006-09-24 17:39     ` Petr Baudis
  0 siblings, 1 reply; 4+ messages in thread
From: Jim Meyering @ 2006-09-16 15:10 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: git

Jonas Fonseca <fonseca@diku.dk> wrote:
> Jim Meyering <jim@meyering.net> wrote Thu, Aug 24, 2006:
>> I tried cg-commit with a commit hook, but the hook never ran.
>> The problem was a typo:
>>
>> Fix a typo that would inhibit running the post-commit script:
>> s/commit-post/post-commit/.
>
> If I remember correctly, historically, the commit-post existed before
> the post-commit appeared. You can see that it is documented in the man
> page so it is not a typo.
>
> However, this should certainly be updated, but I think a better fix
> would be to transitionally warn the user about the existence of the
> commit-post hook before using it in favour of post-commit.

How about this?
Check for both and if there's exactly one, use that.
If it's the old one, give a diagnostic suggesting to rename it.
If both exist, execute neither, give a diagnostic and fail.

If you like this, I'll prepare a patch.

>> diff --git a/cg-commit b/cg-commit
>> index 9d3b1a1..82eea60 100755
>> --- a/cg-commit
>> +++ b/cg-commit
>> @@ -604,15 +604,16 @@ if [ "$newhead" ]; then
>>  		branchname="$(cat "$_git/branch-name")"
>>  	fi
>>  	[ -z "$branchname" ] && [ "$_git_head" != "master" ] && branchname="$_git_head"
>> -	if [ -x "$_git/hooks/post-commit" -a ! "$no_hooks" ]; then
>> +	post_commit="$_git/hooks/post-commit"
>> +	if [ -x "$post_commit" -a ! "$no_hooks" ]; then
>>  		if [ "$(git-repo-config --bool cogito.hooks.commit.post.allmerged)" = "true" ]; then
>>  			# We just hope that for the initial commit, the user didn't
>>  			# manage to install the hook yet.
>>  			for merged in $(git-rev-list $newhead ^$oldhead | tac); do
>> -				"$_git/hooks/post-commit" "$merged" "$branchname"
>> +				"$post_commit" "$merged" "$branchname"
>>  			done
>>  		else
>> -			"$_git/hooks/post-commit" "$newhead" "$branchname"
>> +			"$post_commit" "$newhead" "$branchname"
>>  		fi
>>  	fi
>
> The patch looks more like a refactoring of a previous commit that did
> the commit-post -> post-commit replacement.

Exactly :)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cg-commit: Fix a typo that would inhibit running of post-commit script:
  2006-09-16 15:10   ` Jim Meyering
@ 2006-09-24 17:39     ` Petr Baudis
  0 siblings, 0 replies; 4+ messages in thread
From: Petr Baudis @ 2006-09-24 17:39 UTC (permalink / raw)
  To: Jim Meyering; +Cc: Jonas Fonseca, git

Dear diary, on Sat, Sep 16, 2006 at 05:10:36PM CEST, I got a letter
where Jim Meyering <jim@meyering.net> said that...
> Jonas Fonseca <fonseca@diku.dk> wrote:
> > Jim Meyering <jim@meyering.net> wrote Thu, Aug 24, 2006:
> >> I tried cg-commit with a commit hook, but the hook never ran.
> >> The problem was a typo:
> >>
> >> Fix a typo that would inhibit running the post-commit script:
> >> s/commit-post/post-commit/.
> >
> > If I remember correctly, historically, the commit-post existed before
> > the post-commit appeared. You can see that it is documented in the man
> > page so it is not a typo.
> >
> > However, this should certainly be updated, but I think a better fix
> > would be to transitionally warn the user about the existence of the
> > commit-post hook before using it in favour of post-commit.
> 
> How about this?
> Check for both and if there's exactly one, use that.
> If it's the old one, give a diagnostic suggesting to rename it.
> If both exist, execute neither, give a diagnostic and fail.
> 
> If you like this, I'll prepare a patch.

That would be awesome. :-)

Thanks in advance,

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2006-09-24 17:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-08-24 18:19 [PATCH] cg-commit: Fix a typo that would inhibit running of post-commit script: Jim Meyering
2006-08-25  0:20 ` Jonas Fonseca
2006-09-16 15:10   ` Jim Meyering
2006-09-24 17:39     ` Petr Baudis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).