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

[Junio C Hamano <gitster@pobox.com>]

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-<.

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.