Re: [PATCH] read-cache: hide refresh_cache_entry()

[Patrick Steinhardt <ps@pks.im>]

On Fri, Oct 30, 2026 at 08:46:33AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> >> Hide the low-level implementation detail again by making it a
> >> file-scope static function.  The external callers would indirectly
> >> call it when they call make_cache_entry() or rename_index_entry_at()
> >> external interface, which "ort" (the successor of "recursive") does.
> >
> > Seeing all these patches dropping public declarations I wonder whether
> > they should have been part of the same patch series :)
> 
> Perhaps ;-).
> 
> >> +static struct cache_entry *refresh_cache_entry(struct index_state *, struct cache_entry *, unsigned int);
> >> +
> >>  static const char *alternate_index_output;
> >>  
> >>  static void set_index_entry(struct index_state *istate, int nr, struct cache_entry *ce)
> >
> > Could we avoid this forward declaration by moving the function around?
> 
> As refresh_cache_entry() that has mere one line in its body calls
> refresh_cache_ent() that is a screenful and defined nearby, both of
> them need to be moved upward, so it becomes a rather unsightly
> change, even though nobody would care when they only look at the end
> result X-<.

Well, people probably know by now that I myself tend to err more on the
side of fixing up stuff, even if it's a bit more on the painful side of
things. So I'm not going to surprise you by saying that I think that
would be worth it.

In any case, I don't feel strongly about it.

> By the way, I tried "history fixup" to make this change, and noticed
> that the subcommand ignores "[notes] rewriteref = refs/notes/amlog",
> just like "git history reword" does.  Again, I wonder if this is a
> known limitation, or a deliberate design.

Yeah, I already replied in the other series, this is a known limitation.
I've created an issue to track this that we may have capacity to work on
in the next release cycle. But I wouldn't mind anybody else beating us
to it.

Patrick