From: Junio C Hamano Date: Tue, 12 May 2015 21:21:12 GMT Subject: Re: [PATCH v2 06/18] report_refname_conflict(): inline function Message-ID: In-Reply-To: <1431357920-25090-7-git-send-email-mhagger@alum.mit.edu> Michael Haggerty writes: > It wasn't pulling its weight. And we are about to need code similar to > this where no ref_entry is available and with more diverse error > messages. Rather than try to generalize the function, just inline it. > > Signed-off-by: Michael Haggerty > --- I am not sure "not pulling its weight" in this and also 05/18 is a fair judgement. A short helper function sometimes helps readability by giving an extra abstraction. But I agree 100% with these two steps that they weren't making them easier to read. A raw call to error() shows clearly that we are reporting an error and more importantly by losing an intermediary we do not have to wonder at the call site if there is anything else going on in that "report" function (where there isn't). Thanks. > refs.c | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) > > diff --git a/refs.c b/refs.c > index 6bdd34f..7422594 100644 > --- a/refs.c > +++ b/refs.c > @@ -857,12 +857,6 @@ static int nonmatching_ref_fn(struct ref_entry *entry, void *vdata) > return 1; > } > > -static void report_refname_conflict(struct ref_entry *entry, > - const char *refname) > -{ > - error("'%s' exists; cannot create '%s'", entry->name, refname); > -} > - > /* > * Return true iff a reference named refname could be created without > * conflicting with the name of an existing reference in dir. If > @@ -918,7 +912,7 @@ static int is_refname_available(const char *refname, > */ > return 1; > } > - report_refname_conflict(entry, refname); > + error("'%s' exists; cannot create '%s'", entry->name, refname); > return 0; > } > > @@ -969,7 +963,7 @@ static int is_refname_available(const char *refname, > if (!do_for_each_entry_in_dir(dir, 0, nonmatching_ref_fn, &data)) > return 1; > > - report_refname_conflict(data.found, refname); > + error("'%s' exists; cannot create '%s'", data.found->name, refname); > return 0; > }