Re: [PATCH 1/4] push options: {pre,post}-receive hook learns about push options

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

 



On Fri, Jul 1, 2016 at 12:14 AM, Jeff King <peff@xxxxxxxx> wrote:
>
> Putting the data into a file does alleviate some issues. But it also
> makes parsing annoying, and opens up new issues (like what happens
> to content that has a newline in it?). Wouldn't multiple files be a bit
> more convenient? Especially for your example of eventually carrying
> larger binary content like images.

Yeah I did not think about having multiple files. I think that is the
way to go now.
In the environment variable you'd have the paths, such that

process_hook_option_from_file()
{
 ... # do a thing with $1
}

for p in $GIT_HOOK_OPTION_FILES ;
do
  process_hook_option_from_file $p
done

is a viable hook.


>> The rationale for keeping the actual options inside a file instead of
>> putting them directly into an environment variable has multiple reasons:
>
> Thanks for including this rationale; my first thought on seeing the
> patch was "wouldn't this be much more convenient for a hook if each
> value had its own environment variable?".

That's what I thought as well. Office discussion ensued and I am still
offended by this solution, but it sucks less than multiple environment variables
(I tried writing a script to construct and evaluate the environment
variables and
that doesn't look nice)

If we did not have a GIT_PUSH_OPTIONS_COUNT and GIT_PUSH_OPTION_<N>
but rather GIT_PUSH_OPTIONS_VARIABLES that contains the other variables,
it may be easier to handle, but whether you read from a file or evaluate the
environment variable is only a minor step, the indirection is there anyway
and this would be very close to what we have above.

Thanks,
Stefan

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