From: jayesh0104 <jayeshdaga99@gmail.com>
To: gitster@pobox.com
Cc: ayu.chandekar@gmail.com, git@vger.kernel.org,
gitgitgadget@gmail.com, jayeshdaga99@gmail.com,
jltobler@gmail.com, siddharthasthana31@gmail.com,
stolee@gmail.com
Subject: Re: [PATCH] read-cache: use index state repository for trace2 logging
Date: Sat, 28 Mar 2026 05:25:04 +0000 [thread overview]
Message-ID: <20260328052505.76445-1-jayeshdaga99@gmail.com> (raw)
In-Reply-To: <xmqqqzp5kzj3.fsf@gitster.g>
Hi Junio, Derrick,
Thanks for the detailed review and suggestions.
On the fallback to `the_repository`: I agree with your observation that `istate->repo` being NULL would indicate a bug rather than a scenario to defensively handle. My initial intent was to be conservative in case there were edge paths where `istate->repo` might not be initialized, but given that INDEX_STATE_INIT(r) sets this unconditionally, it makes sense to rely on that invariant instead of masking potential issues. I will drop the fallback and use `istate->repo` directly (and verify via the test suite).
Regarding scope, Derrick’s suggestion to split this into separate commits makes sense. I’ll proceed as follows:
1. A focused patch that replaces `the_repository` with `istate->repo` in the trace2 calls within this file and removes the associated TODO comments.
2. A follow-up patch that replaces other uses of `the_repository` in places where an `istate` is already available (e.g., `refresh_index()`, `tweak_untracked_cache()`, `do_write_index()`, etc.), keeping changes logically grouped for easier review.
I’ll also run the full test suite after removing the fallback to confirm there are no hidden assumptions.
Junio, thanks also for pointing out the author identity. I’ll update it to use my real name in the next version.
Thanks again for the guidance.
Best,
Jayesh
next prev parent reply other threads:[~2026-03-28 5:26 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-27 10:08 [PATCH] read-cache: use index state repository for trace2 logging Jayesh Daga via GitGitGadget
2026-03-27 13:48 ` Derrick Stolee
2026-03-27 16:28 ` Junio C Hamano
2026-03-28 5:25 ` jayesh0104 [this message]
2026-03-28 7:14 ` [PATCH v2] read-cache: use istate->repo " Jayesh Daga via GitGitGadget
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=20260328052505.76445-1-jayeshdaga99@gmail.com \
--to=jayeshdaga99@gmail.com \
--cc=ayu.chandekar@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=jltobler@gmail.com \
--cc=siddharthasthana31@gmail.com \
--cc=stolee@gmail.com \
/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