Hi Junio, On 6 Jun 2024, at 14:21, Junio C Hamano wrote: > ADMINISTRIVIA. Check the address you place on the CC: line. What > we can see for this message at > > https://lore.kernel.org/git/011c10f488610b0a795a843bff66723477783761.1717694801.git.gitgitgadget@xxxxxxxxx/raw > > looks like this. > > Cc: "Phillip Wood [ ]" <phillip.wood123@xxxxxxxxx>, > Kristoffer Haugsbakk <[code@xxxxxxxxxxxxxxx]>, > "Jeff King [ ]" <peff@xxxxxxxx>, > "Patrick Steinhardt [ ]" <ps@xxxxxx>, > "=?UTF-8?Q?Jean-No=C3=ABl?= Avila [ ]" <avila.jn@xxxxxxxxx>, > John Cai <johncai86@xxxxxxxxx>, > John Cai <johncai86@xxxxxxxxx> > > I fixed them manually, but it wasn't pleasant. I think we saw a > similar breakage earlier coming via GGG, but I do not recall the > details of how to cause such breakages (iow, what to avoid repeating > this). oof, apologies. Didn't notice that. I'll be more mindful about the cc line. > > Anyway. > > "John Cai via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > >> 29 files changed, 64 insertions(+), 52 deletions(-) > > Wow, the blast radius of this thing is rather big. Among these > existing callers of refs_resolve_ref_unsafe(), how many of them > will benefit from being able to pass a non NULL parameter at the end > of the series, and more importantly, in the future to take advantage > of the new feature possibly with a separate series? > > I am assuming that this will benefit only a selected few and the > callers that would want to take advantage of the new feature will > remain low. Have you considered renaming refs_resolve_ref_unsafe() > to a new name (say, refs_resolve_ref_unsafe_with_referent()) and > implement the new feature (which is only triggered when the new > parameter gets a non NULL value), make refs_resolve_ref_unsafe() a > very thin wrapper that passes NULL to the new thing? > > That way, you do not have to touch those existing callers that will > never benefit from the new capability in the future. You won't risk > conflicting with in flight topics semantically, either. > > Or will they also benefit from the new feature in the future? > > Offhand, I do not know how a caller like this ... > >> diff --git a/add-interactive.c b/add-interactive.c >> index b5d6cd689a1..041d30cf2b3 100644 >> --- a/add-interactive.c >> +++ b/add-interactive.c >> @@ -533,7 +533,7 @@ static int get_modified_files(struct repository *r, >> { >> struct object_id head_oid; >> int is_initial = !refs_resolve_ref_unsafe(get_main_ref_store(the_repository), >> - "HEAD", RESOLVE_REF_READING, >> + "HEAD", NULL, RESOLVE_REF_READING, >> &head_oid, NULL); >> struct collection_status s = { 0 }; >> int i; > > ... would be helped. Good point. I was sensing the same thing as I made all the callsite changes so this feedback makes it clear what we should do. > > Thanks.
