From: Stephan Beyer Date: Mon, 11 Aug 2008 15:13:03 GMT Subject: Re: [PATCH 1/2] merge-recursive: prepare merge_recursive() to be called from builtins 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 , PGP 0x6EDDD207FCC5040F