Thread (1 message) 1 message, 1 author, 2021-08-18

Re: [PATCH v2 4/6] hook: allow running non-native hooks

From: Junio C Hamano <hidden>
Date: 2021-08-18 21:24:27

Emily Shaffer [off-list ref] writes:
quoted
Yes, I see what you mean. Ok. I have been wanting to change the naming
anyways - most functions in hook.h are verb-y ("find hook", "run hooks",
so on) but hook_list stands out as the only noun-y function.

So I considered changing it to "list_hooks" and "list_hooks_gently", to align
with find_hook(_gently)....
I do not claim I am goot at naming (or better than you at it
anyway), but list-hooks sounds to me like it is calling printf()
to show the hooks to the user, not computing a list of hooks and
returning it to the caller.
quoted
I also think that approach would make a callsite easier to understand
than checking for null from hook_list().

  const char *hookname = "my-new-hook";

  /* Here it's pretty clear what the reason for the error was... */
  if (!known_hook(hookname))
    BUG("is hook '%s' in Documentation/githooks.txt?", hookname);
Yes.  The callsite becomes easier to understand, and it separates
the responsibility between the helper to respond to "please give me
list of defined hooks" and its caller that may react to the returned
list with "ok, among these hooks, this does not look kosher for such
and such reason, so I'd die/warn/error" much cleaner.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help