git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4] hooks: propose project configured hooks

From
Albert Cui <albertqcui@gmail.com>
Date
Jun 3, 2021, 20:16 UTC
Message-ID
<CAMbkP-Qkd0EzNvhKLeOU3LCdTDiYKpTmZJqMN5rFDg-WkVMrAw@mail.gmail.com>
In-Reply-To
<20210603033142.353066-1-jonathantanmy@google.com>
On Wed, Jun 2, 2021 at 8:31 PM Jonathan Tan <jonathantanmy@google.com> wrote:
Show 18 quoted lines
>
> > +* After clone, repository suggests hooks to the user
> > +
> > +    ** User receives a non-interactive message advertising hooks available to
> > +    install
> > +
> > +    ** User can see what hooks and commands are being suggested and from what
> > +    remote.
>
> From the implementation point of view, would it be sufficient to just
> advertise that hooks are available? Assuming that the hooks will be
> available from a specially named ref (as stated below), then we would
> only need to inform the user that this ref exists and hooks can be
> inspected using a special command. Likewise for when we fetch and notice
> that the ref now points to a different object. Then, we wouldn't need to
> do any extra fetching upon clone/fetch, saving time and bandwidth, but
> just do so if the user requests it.
>

From a user perspective, I think it's better to not just tell users that hooks are available but also /what/ hooks are available.

That said, that doesn't require getting everything from the ref as the hook command itself might be stored in this ref. So something like this seems sufficient to me: "Origin suggests setting up a `pre-push` hook which runs `pre_push.sh`. To view the contents of `pre_push.sh`, run {special command}."

Show 22 quoted lines
> > +Feature Requirements
> > +~~~~~~~~~~~~~~~~~~~~
> > +
> > +Minimum Feature Set
> > +^^^^^^^^^^^^^^^^^^^
>
> [snip]
>
> > +* The configuration specifies where the hook commands reside
> > +
> > +    ** This could be a path to a script/binary within the repository
> > +
> > +    ** This could be a path to a script/binary contained within submodules of
> > +    the repository
> > +
> > +    ** This could be a user installed command or script/binary that exists
> > +    outside of the repository and is present in `$PATH`
>
> Right now hooks are fixed files (well, not counting Emily Shaffer's work
> on config hooks). Would it be sufficient to just provide replacements
> for those files?
>

My thought was we'd leverage config hooks and basically write to the config for setting up hooks.

Show 12 quoted lines
> > +* The configuration lives outside the worktree.
> > +
> > +    ** Allows updated hooks to apply across history and branches, reducing
> > +    the maintenance burden of keeping hooks updated.
> > +
> > +    ** Allows different remotes to specify different configuration. This is
> > +    useful for public vs private central repositories or for repositories which
> > +    support different Git functionality.
>
> Hmm...what would be a use case of this? And how would, say, a pre-commit
> hook know which remote it is for?
>

For a concrete example, we use Gerrit for internal reviews and need the Change-Id hook, but we don't want that when upstreaming.

Good question for specifying remotes. This might imply the need for something like `git commit --hooks-from=origin`.

Show 20 quoted lines
> > +* The user receives advice to install hooks.
> > +
> > +    ** The advice should clearly indicate the suggested hook command(s) and hook
> > +    event(s) as well as the central repository that is suggesting them (via
> > +    remote URL).
> > +
> > +    ** The user should be able to use configuration to turn off this advice.
> > +
> > +    ** The advice should appear at the following times:
> > +
> > +        *** After a clone
> > +
> > +        *** After a suggested hook would have run if not already installed. The
> > +        advice should include commands for installing the hook and invoking it.
> > +        For example, for a hook on 'git commit', the user should receive advice
> > +        to amend their commit after hook installation.
>
> This seems contradictory to a point above where we only inform the user
> upon clone (when the user is in the setup mood).
>

Good catch, I should clarify that previous point. The main concern is prompting before a hook will execute which would get in the way of existing workflows, making users susceptible to blindly agreeing. Showing advice after the fact doesn't get in the way, and this is one reason why "advice" felt like the right terminology to use (more below): it's merely a helpful message that a user can optionally choose to follow.

Show 10 quoted lines
> > +* If, after fetch, the central repository suggests new or updated hooks, the
> > +user should receive advice to install these new hooks (note: implementation
> > +should not interfere with requirement listed in“Fast Follows")
>
> In Git, the term "advice" seems to be used more for extra
> explanations that you can turn off once you're experienced with Git.
> Here, these seem like things that we would want to notify users about
> regardless of experience level, so maybe the word "notification" is more
> appropriate.
>

Another reason to use "advice" here is because the existing system allows users to turn off the advice when it's not needed for the requirement: "The user should be able to use configuration to turn off this advice."

Do advice settings work on a per-clone basis? If not, I agree "advice" is probably not the right term.

Show 14 quoted lines
> > +* Works across Windows/Linux/macOS
> > +
> > +Fast Follows
> > +^^^^^^^^^^^^
> > +
> > +* Behind configuration, a user can opt to automatically install hook updates
> > +from a given remote.
> > +
> > +* Allow users to make trust decisions based on GPG signing e.g. if the
> > +configuration came from a signed commit, the signature could be shown along
> > +with the remote it came from.
>
> For the MVP, do we need this?
>
No, that's what I meant by "Fast Follows" -- not needed for the initial feature.
Previous: Jonathan TanNext: Jonathan Tan
Message 38 of 39 in “hooks: propose repository owner configured hooks”
  1. hooks: propose repository owner configured hooksAlbert Cui via GitGitGadget, Mar 18, 2021
  2. Junio C HamanoMar 18, 2021
  3. Albert CuiMar 18, 2021
  4. brian m. carlsonMar 19, 2021
  5. Ævar Arnfjörð BjarmasonMar 19, 2021
  6. Albert CuiApr 6, 2021
  7. Ævar Arnfjörð BjarmasonApr 7, 2021
  8. Jonathan TanJun 21, 2021
  9. Ævar Arnfjörð BjarmasonJun 21, 2021
  10. hooks: propose project configured hooksAlbert Cui via GitGitGadget, Mar 26, 2021
  11. Emily ShafferMar 29, 2021
  12. Albert CuiApr 1, 2021
  13. Derrick StoleeMar 30, 2021
  14. Albert CuiApr 5, 2021
  15. Junio C HamanoApr 5, 2021
  16. Albert CuiApr 5, 2021
  17. Junio C HamanoApr 6, 2021
  18. Albert CuiApr 6, 2021
  19. brian m. carlsonApr 6, 2021
  20. Ævar Arnfjörð BjarmasonApr 7, 2021
  21. Derrick StoleeApr 7, 2021
  22. Albert CuiApr 7, 2021
  23. Junio C HamanoApr 7, 2021
  24. Ævar Arnfjörð BjarmasonApr 7, 2021
  25. Ed MasteApr 15, 2021
  26. Junio C HamanoApr 15, 2021
  27. Ed MasteApr 15, 2021
  28. Junio C HamanoApr 15, 2021
  29. brian m. carlsonApr 15, 2021
  30. Ævar Arnfjörð BjarmasonApr 2, 2021
  31. Albert CuiApr 5, 2021
  32. Ævar Arnfjörð BjarmasonApr 2, 2021
  33. Albert CuiApr 3, 2021
  34. hooks: propose project configured hooksAlbert Cui via GitGitGadget, Apr 24, 2021
  35. Junio C HamanoApr 28, 2021
  36. hooks: propose project configured hooksAlbert Cui via GitGitGadget, May 5, 2021
  37. Jonathan TanJun 3, 2021
  38. Albert CuiJun 3, 2021
  39. Jonathan TanJun 3, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.