opendev/git-review
Code Issues Proposed changes

Fix wrong and misleading "using default: None" in --verbose mode

Browse Source 
When something is not found in git config, the --verbose option logs the
wrong value "None", example:

2018-12-12  Running: git config --get gitreview.branch
2018-12-12  using default: None
2018-12-12  Running: git config --get gitreview.scheme
2018-12-12  using default: None
2018-12-12  Running: git config --get remote.gerrit.pushurl
2018-12-12  using default: None
2018-12-12  Running: git config --get remote.gerrit.url
2018-12-12  result: ssh://marc@review.openstack.org:29418/openstack-infra/git-review

From a --verbose user perspective this is plain wrong for (at least) all the
options defined in the DEFAULTS list.

Change --verbose message to look like this instead:

2018-12-12  Running: git config --get gitreview.branch
2018-12-12  Config['branch'] = master
2018-12-12  Running: git config --get gitreview.scheme
2018-12-12  Config['scheme'] = ssh
2018-12-12  Running: git config --get remote.gerrit.pushurl
2018-12-12  Running: git config --get remote.gerrit.url
2018-12-12  ... remote.gerrit.url = ssh://marc@review.openstack.org:29418/openstack-infra/git-review

If git_config_get_value('new_option',... ) is ever invoked in the future
with a not None, default="fubar" parameter then --verbose will print these
lines:

2018-12-19  Running: git config --get gitreview.new_option
2018-12-19  ... nothing in git config, returning func parameter: fubar

This logging issue is especially misleading considering the many levels of
defaults and fallbacks: git config x3; .gitreview; DEFAULTS list,
git_config_get_value(default=...) parameter, options parser logic, etc.

Change-Id: I6cee46e88b90b8f11689be3875d64ec5e577f12f
This commit is contained in:
Marc Herbert 2018-12-12 16:09:40 -08:00 committed by MarcH
parent dfb37fa0e0
commit d5f02d61cb
1 changed files with 13 additions and 3 deletions

16
git_review/cmd.py

@ -261,12 +261,15 @@ def git_config_get_value(section, option, default=None, as_bool=False):
try:
result = run_command_exc(GitConfigException, *cmd).strip()
if VERBOSE:
print(datetime.datetime.now(), "result:", result)
print(datetime.datetime.now(), "... %s.%s = %s"
% (section, option, result))
return result
except GitConfigException as exc:
if exc.rc == 1:
if VERBOSE:
print(datetime.datetime.now(), "using default:", default)
if VERBOSE and default is not None:
print(datetime.datetime.now(),
"... nothing in git config, returning func parameter:",
default)
return default
raise
@ -289,9 +292,16 @@ class Config(object):
self.config.update(load_config_file(filename))
def __getitem__(self, key):
"""Let 'git config --get' override every Config['key'] access"""
value = git_config_get_value('gitreview', key)
if value is None:
value = self.config[key]
# "--verbose" doesn't trace *early* invocations; for that you
# must change the value at the top of this file (*and* pass
# --verbose)
if VERBOSE:
print(datetime.datetime.now(),
"Config['%s'] = %s " % (key, value))
return value