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

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Aug 30, 2023, 13:24 UTC
Message-ID: <2ad36ef4-4bfa-4e2a-956e-a25d80ce25fd@gmail.com>
In-Reply-To: <20230829234339.GA227087@coredump.intra.peff.net>

On 30/08/2023 00:43, Jeff King wrote:

Show 12 quoted lines

> 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