From: Taylor Blau <me@ttaylorr.com>
To: Kousik Sanagavarapu <five231003@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [RFC PATCH 0/3] pretty, builtin/mailinfo: don't rely on the_repository
Date: Tue, 15 Oct 2024 15:51:39 -0400 [thread overview]
Message-ID: <Zw7Hy12qHhd5BhGh@nand.local> (raw)
In-Reply-To: <20241015144935.4059-1-five231003@gmail.com>
On Tue, Oct 15, 2024 at 08:01:21PM +0530, Kousik Sanagavarapu wrote:
> 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
next prev parent reply other threads:[~2024-10-15 19:51 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-15 14:31 [RFC PATCH 0/3] pretty, builtin/mailinfo: don't rely on the_repository Kousik Sanagavarapu
2024-10-15 14:31 ` [PATCH 1/3] repository: move git_*_encoding configs to repo scope Kousik Sanagavarapu
2024-10-15 16:05 ` shejialuo
2024-10-16 6:15 ` Patrick Steinhardt
2024-10-16 17:24 ` Kousik Sanagavarapu
2024-10-17 5:03 ` Patrick Steinhardt
2024-10-17 13:06 ` shejialuo
2024-10-16 16:31 ` Kousik Sanagavarapu
2024-10-15 14:31 ` [PATCH 2/3] pretty: don't rely on "the_repository" Kousik Sanagavarapu
2024-10-15 14:31 ` [PATCH 3/3] builtin/mailinfo: " Kousik Sanagavarapu
2024-10-15 16:14 ` shejialuo
2024-10-15 19:51 ` Taylor Blau [this message]
2024-10-16 17:49 ` [RFC PATCH 0/3] pretty, builtin/mailinfo: don't rely on the_repository Kousik Sanagavarapu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zw7Hy12qHhd5BhGh@nand.local \
--to=me@ttaylorr.com \
--cc=five231003@gmail.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).