On Mon, Apr 10, 2017 at 07:51:23PM +0100, Daniel Silverstone wrote:
On Sat, Apr 08, 2017 at 15:30:22 +0100, Richard Maw wrote:
> +and the `refs/gitano-admin` branch of each other repository,
that should be `refs/gitano/admin`
> +Per-repository hooks in the `refs/gitano-admin` branch
Ditto
]:¬( I second-guessed myself about which way around it was
and looked at elsewhere in the document where I had it as refs/gitano-admin,
so it looks like I've got more than 2 places to fix.
> +2. `fetch(url, headers, body, content_type)` during
post-receive hooks.
(a) this should be documented further, such as what 'headers' is etc.
Ok, so headers may be nil or an empty table,
and content_type is separate from headers since it's also optional
so is handy to have special handling positionally,
and the header requires special handling so shouldn't be part of headers.
(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.
> +4. An `info` table as the second parameter to per-repository
hook,
> + containing the following indices:
The info table is not passed positionally into the hook functions. It is,
instead passed in a global called 'actor' (see the supple core function in
lib/gitano/supple.lua)
Bit of a reading comprehension failure there.
> +5. The `refname`, `oldsha` and `newsha` as the third to fifth
parameters
> + during per-repository update hooks, as described in [githooks(5)][].
> +
> + Checking whether the master ref is deleted would be accomplished by:
> +
> + local _, _, refname, oldsha, newsha = ...
> + if refname == "refs/heads/master" and newsha ==
("0"):rep(40) then
> + log.state("Master deleted")
> + end
I think that'd be local _, refname, oldsha, newsha (as per the above note)
Yep.
> +6. For pre-receive or post-receive per-repository hooks as the
third parameter,
> + a table indexed by the `refnames` being modified
> + containing tables of the `oldsha` and `newsha` indexed
> + either at positional indices `1` and `2`,
> + or by name `oldsha` and `newsha`.
> + Checking whether the master ref is deleted would be accomplished by:
> +
> + local _, _, updates = ...
local _, updates = ...
Yep.
> + if (updates["refs/heads/master"] or
{}).newsha == ("0"):rep(40) then
> + log.state("Master deleted")
> + end
> +
> +7. Global hooks work the same as per-repository hooks,
> + except their first parameter is instead a callable function
> + which will run the per-repository hook when called,
> + so global hooks may either replace or augment per-repository hooks.
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.