Re: [RFC PATCH 0/3] pretty, builtin/mailinfo: don't rely on the_repository

From: Taylor Blau <me@ttaylorr.com>
Date: Oct 15, 2024, 19:51 UTC
Message-ID: <Zw7Hy12qHhd5BhGh@nand.local>
In-Reply-To: <20241015144935.4059-1-five231003@gmail.com>

On Tue, Oct 15, 2024 at 08:01:21PM +0530, Kousik Sanagavarapu wrote:

Show 19 quoted lines

> Hi,
> Just a brief summary -
>
> 1/3 - the main changes are in environment.[ch] and repository.[ch], all
>       the others are just changes due to this change.
>
> 2/3 - the main changes are in pretty.[ch], all the other changes are due
>       to this change.
>
> 3/3 - This is pretty straight-forward.
>
> One may notice that there are more "the_repository" occurences now than
> before this change - which is good since it means that we have now made
> the respective dependencies explicit (these were previously implicit).
>
> The change in 1/3 is marked RFC since I was kind of skeptical about the
> "repo" check in the repo_*() functions being done at _that_ level.
> Since every other change in this series depends on this, I've marked all
> the other RFC as well.

I share the concern that others have raised in this thread about not having the_repository when one of the affected commands is ran outside of the repository.

I'll bring these patches into my tree, but let's hold off on queueing them into 'seen' for now.

In the meantime, as a style suggestion, it might be nice to provide a wrapper for function foo() -> repo_foo(), where the former still exists, but is a wrapper for repo_foo(the_repository) like we have done in other similar transitions.

Thanks, Taylor