On 04/07/2017 01:20 PM, Duy Nguyen wrote: > On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <[email protected]> > wrote: >> It turns out that we can now implement >> `refs_verify_refname_available()` based on the other virtual >> functions, so there is no need for it to be defined at the backend >> level. Instead, define it once in `refs.c` and remove the >> `files_backend` definition. >> >> Signed-off-by: Michael Haggerty <[email protected]> >> --- >> refs.c | 85 >> ++++++++++++++++++++++++++++++++++++++++++++++++++-- >> refs.h | 2 +- >> refs/files-backend.c | 39 +++++------------------- > > Much appreciated. This will make future backends simpler to implement as well. > >> + iter = refs_ref_iterator_begin(refs, dirname.buf, 0, >> + DO_FOR_EACH_INCLUDE_BROKEN); >> + while ((ok = ref_iterator_advance(iter)) == ITER_OK) { >> + if (skip && >> + string_list_has_string(skip, iter->refname)) >> + continue; >> + >> + strbuf_addf(err, "'%s' exists; cannot create '%s'", >> + iter->refname, refname); >> + ok = ref_iterator_abort(iter); > > Saving the return code in "ok" seems redundant because you don't use > it. Or did you want to check that ok == ITER_DONE or die() too?
True, setting `ok` here is redundant. I don't think there's much point worrying about whether `ref_iterator_abort()` fails here, since we've already gotten the information that we require. I'll remove it. Thanks, Michael
