Re: [PATCH 0/9] remove unnecessary if statement
- From
Usman Akinyemi <usmanakinyemi202@gmail.com>
- Date
- Mar 19, 2025, 03:53 UTC
- Message-ID
- <CAPSxiM9pW4V5_TxttNrkEKhgv-qaqgRGDgr+jkSCA0vNj71NsA@mail.gmail.com>
- In-Reply-To
- <xmqqmsdi144i.fsf@gitster.g>
On Wed, Mar 19, 2025 at 1:51 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 19 quoted lines
> > Usman Akinyemi <usmanakinyemi202@gmail.com> writes: > > > In an earlier patch[1] which has been merged to the master, > > We checked `repo` is not NULL before making call to `repo_config()`. > > Later, in another patch series[2] which has been merged to next, > > `repo_config()` was taught to allow `repo` to be NULL. > > > > So there is not need for checking if the `repo` is NULL before calling > > repo_config() in the earlier patch. > > OK, that sounds good. > > Are we confident that our half-hearted choice of "there is no repo, > so just do a very-early-config thing" is appropriate for any code > paths? > > At least we should be perfectly happy with that choice applied to > all of these code paths touched by this series.
This is fine for the commands which use only RUN_SETUP. I also picked and checked those code paths touched by this series to ensure they do what we intend what we intend them to do. I mean, if there is any better option, that would be cool.
Show 8 quoted lines
> > > Note, I have already sent the first 8 patches in [2] but, the 9th > > patch depends on the first patch of that series. > > So, is this [v2 0/9] of ua/some-builtins-wo-the-repository? > > I think that topic has long been merged to 'next', and it is way too > late to do a wholesale replacement like this.
It is not a replacement for that patch series but, the last patch here 9/9 depends on the first patch in that series. That is why I sent everything together.
Thank you.
Show 33 quoted lines
> > > [1] https://public-inbox.org/git/20250210181103.3609495-1-usmanakinyemi202@gmail.com/ > > [2] https://public-inbox.org/git/20250307233543.1721552-1-usmanakinyemi202@gmail.com/ > > > > Usman Akinyemi (9): > > config: teach repo_config to allow `repo` to be NULL > > builtin/verify-tag: stop using `the_repository` > > builtin/verify-commit: stop using `the_repository` > > builtin/send-pack: stop using `the_repository` > > builtin/pack-refs: stop using `the_repository` > > builtin/ls-files: stop using `the_repository` > > builtin/for-each-ref: stop using `the_repository` > > builtin/checkout-index: stop using `the_repository` > > builtin/update-server-info: remove unnecessary if statement > > > > builtin/checkout-index.c | 43 ++++++++++++++++----------------- > > builtin/for-each-ref.c | 5 ++-- > > builtin/ls-files.c | 32 ++++++++++++------------ > > builtin/pack-refs.c | 8 +++--- > > builtin/send-pack.c | 7 +++--- > > builtin/update-server-info.c | 4 +-- > > builtin/verify-commit.c | 13 +++++----- > > builtin/verify-tag.c | 7 +++--- > > config.c | 4 +++ > > config.h | 9 +++++++ > > t/t0610-reftable-basics.sh | 7 ++++++ > > t/t2006-checkout-index-basic.sh | 7 ++++++ > > t/t3004-ls-files-basic.sh | 7 ++++++ > > t/t5400-send-pack.sh | 7 ++++++ > > t/t6300-for-each-ref.sh | 7 ++++++ > > t/t7030-verify-tag.sh | 7 ++++++ > > t/t7510-signed-commit.sh | 7 ++++++ > > 17 files changed, 118 insertions(+), 63 deletions(-)