Re: [PATCH] add a pre-merge hook

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> I think we should have done something like this for post_merge, not just
> "1/0 to show if it is a squash" which was an ill-thought-out hack.  Is
> there a clean way to fix the interface without breaking existing
> post_merge hooks in people's repositories?

Well, you can add this argument as a second argument to post-merge.
Shall I do it?

> Please call whatever you install from template/hooks--* a "sample", not
> "default".

Ok.

> If it is primarily a hack to support calling post-merge with an non-object
> name "0" or "1"

It is to support not appending the commit list to the arguments of
post-merge.

>> +static int run_pre_merge_hook(const char *kind)
>> +{
>> +	return run_hook("pre-merge", remoteheads,
>> +			squash ? "squash" : kind, "--", NULL);
>>  }
> 
> remoteheads is a commit_list, you have the name of the hook and kind, so
> this extra "--" is only for run_hook() hackery to distinguish between the
> callers to post-merge and the callers to new pre-merge?

No, absolutely.  The "--" separator is for future extension, in case we
want to add other arguments before the head list.

run_hook does not care about what it receives in its varargs part.  It
is copied verbatim until the NULL.  Then it adds a "--" and the heads
argument if requested (which is not the case for post-merge).  The fact
that I had to specify a "--" here though is fishy.  Maybe I have a
off-by-one somewhere that, if fixed, can simplify the code somehow.

I'll take a look tomorrow, thanks for the review.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux