All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phil Hord <hordp@cisco.com>
To: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>,
	Phil Hord <phil.hord@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Erik Faye-Lund <kusmabite@gmail.com>
Subject: Re: [PATCHv3 2/5] Learn to handle gitfiles in enter_repo
Date: Thu, 06 Oct 2011 15:16:31 -0400	[thread overview]
Message-ID: <4E8DFE8F.3060408@cisco.com> (raw)
In-Reply-To: <CACsJy8BiZ7Ey95BOf4p-zwysyYwEY6WectRaj-GqnFvgDNTtZw@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> says,
> On Thu, Oct 6, 2011 at 12:31 AM, Phil Hord <hordp@cisco.com> wrote:
>> -               if (!suffix[i] || chdir(used_path))
>> +               if (!suffix[i])
>> +                       return NULL;
>> +               gitfile = read_gitfile(used_path) ;
>> +               if (gitfile)
>> +                       strcpy(used_path, gitfile);
>> +               if (chdir(used_path))
>>                        return NULL;
>>                path = validated_path;
>>        }
> 
> This is room for improvement, the patch is fine as it is now. We could
> improve error reporting here. If .git file points to nowhere, we get
> "not a repository-kind of message. Except daemon.c, enter_repo()
> callers always die() if enter_repo() returns NULL. We could move the
> die() part (with improved error message) into enter_repo().
> 
> We could update enter_repo(const char *, int) to enter_repo(const char
> *, int, int gently). If gently is 1, we never die() nor report
> anything (ie. what we're doing now). daemon.c will need this, the rest
> of callers will be happy with gently = 0.

I like that.  It wasn't clear to me what the 'gently' moniker meant
before, but now I understand it.  It could easily apply to this function
and the new is_gitfile() to help reduce code duplication.

In a different patch, though.

Phil

      reply	other threads:[~2011-10-06 19:16 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-05 13:31 [PATCHv3 2/5] Learn to handle gitfiles in enter_repo Phil Hord
2011-10-06  3:11 ` Nguyen Thai Ngoc Duy
2011-10-06 19:16   ` Phil Hord [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E8DFE8F.3060408@cisco.com \
    --to=hordp@cisco.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kusmabite@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=phil.hord@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.