On Wed, Jun 14, 2017 at 11:07:26AM +0200, Michael Haggerty wrote: > `for_each_bisect_ref()` is called by `for_each_bad_bisect_ref()` with > a term "bad". This used to make it call `for_each_ref_in_submodule()` > with a prefix "refs/bisect/bad". But the latter is the name of the > reference that is being sought, so the empty string was being passed > to the callback as the trimmed refname. Moreover, this questionable > practice was turned into an error by > > b9c8e7f2fb prefix_ref_iterator: don't trim too much, 2017-05-22 > > It makes more sense (and agrees better with the documentation of > `--bisect`) for the callers to receive the full reference names. So > > * Add a new function, `for_each_fullref_in_submodule()`, to the refs > API.
You might want to mention that this is really just a hole in the existing API. We have for_each_ref_in_submodule() and for_each_fullref_in(), but not the missing link. I don't think that makes it any more or less correct, but I thought at first you had to invent a new function totally. > * Change `for_each_bad_bisect_ref()` to call the new function rather > than `for_each_ref_in_submodule()`. > > Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> > --- > refs.c | 12 ++++++++++++ > refs.h | 5 ++++- > revision.c | 2 +- > 3 files changed, 17 insertions(+), 2 deletions(-) The change itself looks fine to me. Since we obviously don't have even a single test for "--bisect", that might be worth adding. -Peff
