Re: [PATCH] contrib/hooks/post-receive-email: Make revision display configurable

[Pete Harlan <pgit@pcharlan.com>]

Junio C Hamano wrote:
> Pete Harlan <pgit@pcharlan.com> writes:
> 
>> Add configuration option hooks.showrev, letting the user override how
>> revisions will be shown in the commit email.
>>
>> Signed-off-by: Pete Harlan <pgit@pcharlan.com>
>> Acked-By: Andy Parkins <andyparkins@gmail.com>
>> @@ -390,8 +396,13 @@ generate_update_branch_email()
>>  
>>  		echo ""
>>  		echo $LOGBEGIN
>> -		git rev-parse --not --branches | grep -v $(git rev-parse $refname) |
>> -		git rev-list --pretty --stdin $oldrev..$newrev
>> +		git rev-parse --not --branches |
>> +			grep -v $(git rev-parse $refname) |
>> +			git rev-list --stdin $oldrev..$newrev |
>> +		while read onerev
>> +		do
>> +			eval $(printf "$showrev" $onerev)
>> +		done
> 
> This would make the default "non-custom" case much less efficient.  If we
> really cared, perhaps we could do something like this...
> 
> 	git rev-parse --not --branches |
>         grep -v "^$(git-rev-parse --not $refname)\$" |
>         case "$using_custom_showrev" in
>         yes)
> 		git rev-list --stdin $oldrev..$newrev |
>         	while read onerev
>                 do
>                 done
>                 ;;
> 	*)
>         	git rev-list --pretty --stdin $oldrev..newrev
>                 ;;
> 	esac
> 
> But I suspect we do not care too much about it.

I agree about the efficiency; that bothered me when I wrote the patch,
but figured the lack of efficiency wouldn't be noticeable.

OTOH, it still bothers me; I'll write up a v2 patch that does something
much more like what you suggest.

Is it easier at this point for you to have a new patch that applies on
top of my previous patch, or a wholesale replacement for it?  Or is
what's done done and that's the end of it for something this minor?

Thanks to you both for your time and work,

--Pete

> 
> Will apply as submitted.  Thanks
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html