Re: [PATCHv5 5/8] clone: factor out checking for an alternate path

From: Jacob Keller <jacob.keller@gmail.com>
Date: Aug 23, 2016, 22:29 UTC
Message-ID: <CA+P7+xrd4eagXSFQen+TZop7u+h2eyLgQ_0vjT03Bjov-KNL7w@mail.gmail.com>
In-Reply-To: <20160815215327.15682-6-sbeller@google.com>

On Mon, Aug 15, 2016 at 2:53 PM, Stefan Beller <sbeller@google.com> wrote:

Show 5 quoted lines

> +       if (!repo && is_directory(mkpath("%s/.git/objects", ref_git))) {
> +               char *ref_git_git = mkpathdup("%s/.git", ref_git);
> +               free(ref_git);
> +               ref_git = ref_git_git;
> +       } else if (!is_directory(mkpath("%s/objects", ref_git))) {

I realize this was directly copied but the use of !repo to determine what file path here was definitely not easy to process, because it made me think we were adding .git twice to the path. Not sure if there is an easier way to expand this and avoid that confusion?

Also, what happens if repo is false, so we run the first block above, but is_directory fails? We just continue along even though it appears like we should fail? I'm not 100% sure I follow the logic here. Regardless this appears to be a pretty direct copy of what was there before so I don't think it's worse.

Thanks, Jake