Re: [PATCH] Add section on git hooks
On Mon, Apr 10, 2017 at 08:38:55PM +0100, Daniel Silverstone wrote:
On Mon, Apr 10, 2017 at 20:32:18 +0100, Richard Maw wrote:
> > (b) while you're right that it's currently only for post-receive,
I'd
> > be interested in knowing if you think we should relax that?
>
> I assumed it was deliberate to dissuade its use for external authentication
> by fetching from an external service and failing for taking so long.
Fair, I couldn't think of the reasoning but this is sound enough for now.
To be fair, we don't have a story for external authentication sources,
so perhaps we should experiment with how we _could_ do it that way,
perhaps modify the gitano rulesets to allow by default and check via hook.
That said, I don't think you can change the exit code via hook anyway,
so we can't really do anything about it.
> > Is it worth showing therefore a passthrough for one of the
hooks? E.g.
> >
> > -- Example global-hooks/update.lua
> > local hookf, repo, refname, oldsha, newsha = ...
> > -- Do stuff with repo/refname/oldsha/newsha here
> > --
> > -- And finally (optionally) tail-chain through to the per-repo hook
> > return hookf(repo, refname, oldsha, newsha)
>
> Potentially.
>
> I think I avoided doing so for the lack of examples
> of when you would chain the hooks and when you would override though.
>
> For that matter, if you wanted to provide a default hook
> you would have to check whether
repo:get("refs/gitano/admin:hooks/$hook.lua")
> is a file that exists, since hookf defaults to a no-op callable.
I'm afraid I don't follow. Why would you have to check?
If you're defining a default hook
you should call the local hook if it exists rather than your own hook,
if it doesn't exist you should call your default hook implementation,
but you need to know whether your local hook exists.
From a reading of the code it looks like you never get a nil to say
it's absent
you get a callable that does nothing instead, to make the logic
simpler.