Thread (1 message) 1 message, 1 author, 2018-07-18

Re: [PATCH 2/8] refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callback

From: Junio C Hamano <hidden>
Date: 2018-07-18 20:13:12

Stefan Beller [off-list ref] writes:
quoted
 - existing users of for_each_replace_ref() who were all happy
   working in the_repository have to pass it explicitly, even
   thought they do not have any need to.
All callbacks that are passed to for_each_replace_ref now
operate on 'r' instead of the_repository, and that actually fixes
a bug (commit message is lacking), but the cover letter hints at:
...
quoted
In this case, even if you introduced for_each_replace_ref_in_repo(),
making for_each_replace_ref() a thin wrapper that always uses
the_repository is a bit more cumbersome than just a simple macro.
Yes, but such a callback would do the *wrong* subtle thing in some cases
as you really want to work in the correct repository for e.g. looking up
commits.
quoted
But it *is* doable (you'd need to use a wrapping structure around
cb_data), and a developer who case about maintainability during API
transition would have taken pains to do so.  A bit dissapointing.
My original patches were RFC-ish and a trade off as for the reflog only
there is nothing in flight to care about.

Given that we would want to upgrade all the ref callbacks, we have to
take this route, I think.
Try to rebuild 'pu' on top of 'master' yoruself to realize how many
in-flight topics you are hurting and causing unnecessary load on the
maintainer with the "let's add the 'r' parameter without changing
the function names; compiler would catch and cause breakages"
approach.  

Until that happens, I won't waste any more time trying to educate
you on this further.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help