Karthik Nayak <karthik.188@xxxxxxxxx> writes:
> Remove the error reporting variable to make the code easier to port
> over to using ref-filter APIs.
>
> This also removes the error from being displayed. As branch.c will use
> ref-filter APIs in the following patches, the error checking becomes
> redundant with the error reporting system found in the ref-filter
> (ref-filter.c:1336).
I would have written
As branch.c will use ref-filter APIs in the following patches, the error
checking becomes redundant with the error reporting system found in the
ref-filter: error "branch '%s' does not point at a commit" is redundant
with the check performed in ref_filter_handler (ref-filter.c:1336).
Error "some refs could not be read" can only be triggered as a
consequence of the first one hence becomes useless.
> @@ -370,10 +369,8 @@ static int append_ref(const char *refname, const struct object_id *oid, int flag
> commit = NULL;
> if (ref_list->verbose || ref_list->with_commit || merge_filter != NO_FILTER) {
> commit = lookup_commit_reference_gently(oid->hash, 1);
> - if (!commit) {
> - cb->ret = error(_("branch '%s' does not point at a commit"), refname);
> + if (!commit)
> return 0;
> - }
Am I correct that the "return 0" statement above is dead code after the
end of the series?
If so, you should add a comment explaining that it's there "just in
case" but not supposed to happen, or replace the if statement with
"assert(commit);" IMHO. I have a preference for assert(): I don't like
silent failures.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html