Subject: Re: [PATCH v2 01/22] sequencer: use repository parameter in short_commit_name()

From: Phillip Wood <[email-masked]>
Date: Wed, 30 Aug 2023 14:24:12 +0100

On 30/08/2023 00:43, Jeff King wrote:
> Instead of just using the_repository, we can take a repository parameter
> from the caller. Most of them already have one, and doing so clears up a
> few -Wunused-parameter warnings. There are still a few callers which use
> the_repository, but this pushes us one small step forward to eventually
> getting rid of those.
> 
> Note that a few of these functions have a "rev_info" whose "repo"
> parameter could probably be used instead of the_repository. I'm leaving
> that for further cleanups, as it's not immediately obvious that
> revs->repo is always valid, and there's quite a bit of other possible
> refactoring here (even getting rid of some "struct repository" arguments
> in favor of revs->repo).

I think opts->revs is only initialized when we're building to todo list 
for cherry-picking/reverting a sequence of commits so I the scope for 
removing "struct repository" arguments is pretty limited as any function 
that is called by "cherry-pick --continue" or rebase needs a separate 
repository argument. I'm pretty sure your v1 was safe but this version 
is much more obviously safe.

Best Wishes

Phillip