Re: [PATCH 1/7] worktree: move should_prune_worktree() to worktree.c

On Wed, Jan 6, 2021 at 1:55 AM Eric Sunshine [off-list ref] wrote:

On Mon, Jan 4, 2021 at 11:22 AM Rafael Silva
[off-list ref] wrote:

quoted

+/*
+ * Return true if worktree entry should be pruned, along with the reason for
+ * pruning. Otherwise, return false and the worktree's path, or NULL if it
+ * cannot be determined. Caller is responsible for freeing returned path.
+ */

It would be good to update the documentation to explain what `expire`
is since it's not necessarily obvious. The documentation could also be
tweaked to say that the worktree's path is returned in `wtpath` rather
than saying only that it is returned. If you choose to make these
changes, they should be probably done in a separate patch from the
patch which moves the code. This is a very minor issue, not
necessarily worth a re-roll.

On second thought, adding a patch just to make a minor update to the
function comment is probably overkill. It should be okay to do it in
this patch. Mention the change in the commit to alert reviewers.