On Wed, Jan 6, 2021 at 1:55 AM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Mon, Jan 4, 2021 at 11:22 AM Rafael Silva > <rafaeloliveira.cs@xxxxxxxxx> wrote: > > +/* > > + * 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.