public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: "Erik Elfström" <erik.elfstrom@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH/RFC v3 0/4] Improving performance of git clean
Date: Sat, 18 Apr 2015 18:14:17 -0700	[thread overview]
Message-ID: <xmqqfv7wor6e.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1429389672-30209-1-git-send-email-erik.elfstrom@gmail.com> ("Erik Elfström"'s message of "Sat, 18 Apr 2015 22:41:08 +0200")

Erik Elfström <erik.elfstrom@gmail•com> writes:

> Known Problems:
> * Unsure about the setup.c:read_gitfile refactor, feels a bit
>   messy?

The interface indeed feels somewhat messy.  I suspect that a better
interface might be more like setup_git_directory_gently() that is a
gentler version of setup_git_directory().  The function takes an
optional pointer to an int, and it behaves not-gently-at-all when
the optional argument is NULL.  The location the optional pointer
points at, when it is not NULL, can be used to return the error
state to the caller.  That function pair only uses the optional
argument to convey one bit of information (i.e. "are we in any git
repository or not?") back to the caller, but the interface could be
used to tell the caller a lot more if we wanted to.

If you model read_gitfile_gently() after that pattern, I would
expect that

 - The extra pattern would be "int *error";
 - The implementation of read_gitfile() would be

       #define read_gitfile(path) read_gitfile_gently((path), NULL)

   and the _gently() version will die when "error" parameter is set
   to NULL and finds any error.

 - The caller of the gentle variant can use the error code to
   determine what went wrong, not just the fact that it failed.  I
   do not think your caller does not have an immediate need to tell
   between "invalid gitfile format" and "No path in gitfile", but
   such an interface leaves that possibility open.

Thanks.

next prev parent reply	other threads:[~2015-04-19  1:14 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-18 20:41 [PATCH/RFC v3 0/4] Improving performance of git clean Erik Elfström
2015-04-18 20:41 ` [PATCH/RFC v3 1/4] setup: add gentle version of read_gitfile Erik Elfström
2015-04-18 20:41 ` [PATCH/RFC v3 2/4] t7300: add tests to document behavior of clean and nested git Erik Elfström
2015-04-18 20:41 ` [PATCH/RFC v3 3/4] p7300: add performance tests for clean Erik Elfström
2015-04-18 20:41 ` [PATCH/RFC v3 4/4] clean: improve performance when removing lots of directories Erik Elfström
2015-04-19  1:14 ` Junio C Hamano [this message]
2015-04-21 18:17   ` [PATCH/RFC v3 0/4] Improving performance of git clean erik elfström
2015-04-20 22:14 ` Thomas Gummerer
2015-04-21 18:21   ` erik elfström
2015-04-21 19:02     ` Junio C Hamano
2015-04-21 21:24     ` Jeff King
2015-04-22 19:30       ` erik elfström
2015-04-22 19:46         ` Jeff King
2015-04-22 19:53           ` erik elfström

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=xmqqfv7wor6e.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=erik.elfstrom@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox