Re: [PATCH 1/2] merge-recursive: prepare merge_recursive() to be called from builtins

From: Stephan Beyer <s-beyer@gmx.net>
Date: Aug 11, 2008, 15:13 UTC
Message-ID: <20080811151303.GA11208@leksak.fem-net>
In-Reply-To: <3db5bd9a8fa1fefb9bd188cc148db826b77fec12.1218374062.git.vmiklos@frugalware.org>

Hi,

Miklos Vajna wrote:

> When other builtins call merge_recursive(), they would have to handle
> the GIT_MERGE_VERBOSITY environment variable, causing a code
> duplication. Same story for the git_config() call. It's better to do it
> when merge_recursive() is called the first time.

Hmm, I have the long-run vision that we have a nice libgit some day,
with merge_recursive() being part of it.  And I'm a little unsure if
libified functions should rely on environment variables.
So I'm wondering if the verbosity should be set in the caller functions
of merge_recursive(), i.e. that cmd_merge_recursive() and cmd_merge()
(or another part of builtin-merge.c) does the
	getenv("GIT_MERGE_VERBOSITY")
stuff and a verbosity value could be a new argument to merge_recursive().
Then other merge_recursive() users don't need to
	setenv("GIT_MERGE_VERBOSITY", "3", 1)
or something, they just pass 3 as the verbosity value.

Just a thought,
  Stephan

PS: Your patch looks fine to me.

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F