Re: [PATCH] worktree: populate lock_reason in get_worktrees and light refactor/cleanup in worktree files

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Oct 25, 2018, 19:14 UTC
Message-ID: <CAPig+cTvKd2DVu7wW_A31p_o7BaNJszu14kNRz9sqk8h45H4-g@mail.gmail.com>
In-Reply-To: <CAC05386F1X7TsPr6kgkuLWEwsmdiQ4VKTF5RxaHvzpkwbmXPBw@mail.gmail.com>

On Thu, Oct 25, 2018 at 1:47 AM Nickolai Belakovski <nbelakovski@gmail.com> wrote:

Show 9 quoted lines

> The motivation for the change is some work that I'm doing to add a
> worktree atom in ref-filter.c. I wanted that atom to be able to access
> all fields of the worktree struct and noticed that lock_reason wasn't
> getting populated so I figured I'd go and fix that.
>
> Reviewing this work in the context of your feedback and Junio's
> previous comments, I think it makes sense to only have a field in the
> struct indicating whether or not the worktree is locked, and have a
> separate function for getting the reason.

Is your new ref-filter atom going to be boolean-only or will it also have a form (or a separate atom) for retrieving the lock-reason? I imagine both could be desirable.

In any event, implementation-wise, I would think that such an atom (or atoms) could be easily built with the existing worktree API (with its lazy-loading and caching), which might be an easy way forward since you wouldn't need this patch or the updated one you posted[1], thus no need to justify such a change.

> Since the only cases in
> which the reason is retrieved in the current codebase are cases where
> the program immediately dies, caching seems a moot point.

If your new atom has a form for retrieving the lock reason, then caching could potentially be beneficial(?).

[1]: https://public-inbox.org/git/20181025055142.38077-1-nbelakovski@gmail.com/