Re: [PATCH 06/10] setup: refactor repo format reading and verification

From: Duy Nguyen <pclouds@gmail.com>
Date: Mar 3, 2016, 13:19 UTC
Message-ID: <CACsJy8B6j2AB5CCBVVz7f1JShq0XVicJfzZvjRO54ksde0RMfQ@mail.gmail.com>
In-Reply-To: <20160301144206.GF12887@sigill.intra.peff.net>

On Tue, Mar 1, 2016 at 9:42 PM, Jeff King <peff@peff.net> wrote:

Show 12 quoted lines

> -               for (i = 0; i < unknown_extensions.nr; i++)
> -                       warning("unknown repository extension: %s",
> -                               unknown_extensions.items[i].string);
> -               *nongit_ok = -1;
> -               ret = -1;
> +int verify_repository_format(const struct repository_format *format,
> +                            struct strbuf *err)
> +{
> +       if (GIT_REPO_VERSION_READ < format->version) {
> +               strbuf_addf(err, "Expected git repo version <= %d, found %d",
> +                           GIT_REPO_VERSION_READ, format->version);
> +               return -1;

Since you have given lots of thoughts about this code, perhaps just a little bit more, double check if these strings should be translatable or not and mark _() if so?

Show 10 quoted lines

>         }
>
> -       strbuf_release(&sb);
> -       return ret;
> +       if (format->version >= 1 && format->unknown_extensions.nr) {
> +               int i;
> +
> +               for (i = 0; i < format->unknown_extensions.nr; i++)
> +                       strbuf_addf(err, "unknown repository extension: %s",
> +                                   format->unknown_extensions.items[i].string);

Ditto.

-- 
Duy