Thread (13 messages) flat view 13 messages, 4 authors, 2016-06-15

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

From: Stephan Beyer <hidden>
Date: 2016-06-15 22:45:08

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 [off-list ref], PGP 0x6EDDD207FCC5040F
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help