Stefan Beller wrote:

> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -1055,15 +1055,15 @@ static void assure_connectivity_checked(struct 
> command *commands,
>  
>       for (cmd = commands; cmd; cmd = cmd->next) {
>               if (should_process_cmd(cmd) && si->shallow_ref[cmd->index]) {
> -                     error("BUG: connectivity check has not been run on ref 
> %s",
> -                           cmd->ref_name);
> +                     die("BUG: connectivity check has not been run on ref 
> %s",
> +                         cmd->ref_name);

This could stay as error() so that the caller gets to see the list of
refs that didn't experience a connectivity check.  Or if that list
isn't important, this error/die call could be dropped and the
'checked_connectivity = 0' setting would be enough.

>                       checked_connectivity = 0;
>               }
>       }
>       if (!checked_connectivity)
> -             error("BUG: run 'git fsck' for safety.\n"
> -                   "If there are errors, try to remove "
> -                   "the reported refs above");
> +             die("BUG: run 'git fsck' for safety.\n"
> +                 "If there are errors, try to remove "
> +                 "the reported refs above");

I find this error message misleading and confusing.  It makes it seem
like this is an expected error that we are trying to help the user
recover from.  Instead, something impossible has happened.  "Try to
remove the reported refs" is actively harmful advice --- that would be
a way for the user to work around a serious bug instead of figuring
out what went wrong and getting git fixed.

Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to