git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jim Meyering <jim@meyering.net>
To: Jonas Fonseca <fonseca@diku.dk>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] cg-commit: Fix a typo that would inhibit running of post-commit script:
Date: Sat, 16 Sep 2006 17:10:36 +0200	[thread overview]
Message-ID: <87ac4zq237.fsf@rho.meyering.net> (raw)
In-Reply-To: <20060825002013.GG2817@diku.dk> (Jonas Fonseca's message of "Fri, 25 Aug 2006 02:20:13 +0200")

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

  reply	other threads:[~2006-09-16 15:11 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2006-09-24 17:39     ` Petr Baudis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ac4zq237.fsf@rho.meyering.net \
    --to=jim@meyering.net \
    --cc=fonseca@diku.dk \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).