On Fri, Mar 01, 2013 at 11:43:24AM +0000, Sam Thursfield wrote:
On 02/28/2013 06:10 PM, Richard Maw wrote:
>Some subcommands use git to create commits, in which case user config
>needs to be set.
>Others imply commits may be created, e.g. by cloning a repository, at
>which point it is useful to have a reminder that the configuration needs
>to be set.
> morphlib/git.py | 17 +++++++++++++++++
> morphlib/plugins/branch_and_merge_plugin.py | 21 +++++++++++++++++++++
> 2 files changed, 38 insertions(+), 0 deletions(-)
>diff --git a/morphlib/git.py b/morphlib/git.py
>index c63c21b..6768107 100644
>@@ -183,6 +183,23 @@ def get_user_email(runcmd):
> ' git config --global user.email
>+def check_config_set(runcmd, keys=("user.name", "user.email"),
>+ ''' Check whether the given keys have values in git config.
>+ missing = 
>+ for key in keys:
>+ runcmd(['git', 'config', key], cwd=cwd)
>+ except cliapp.AppException:
>+ if missing:
>+ if len(missing) == 1:
>+ emesg = 'Git configuration for %s has not been set' %
>+ emesg = ('Git configuration for keys %s and %s have not been
>+ % (', '.join(missing[:-1]), missing[-1]))
>+ raise cliapp.AppException(emsg)
morphlib.git.get_user_name() and morphlib.git.get_user_email()
already warn if the config is missing. There's no point doing that
if we explicitly check first.
They are an error, not a warning. It is an error to create commits
without the user's config being set.
However, we create repositories in the other functions as well, which
the user may commit in. The requirement for this patch was that in these
cases the user should be warned that they should set their config.
I prefer the approach of checking as early as possible, to avoid
failing half way through a slow operation. However, I also prefer
the warning that get_user_email() gives because it's a bit more
friendly and resembles the warning the git would give a bit more
Fair point, I'm open to the format it should be printed in.