Re: [PATCH 1/2] reflog: drop usage of global variables

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> The 'builtin/reflog.c' file uses the 'the_repository' global variable
> directly and also via 'git_config()'. Since this is a builtin command
> which has access to the 'struct repository', drop usage of the global
> variable and use the available repository struct.
>
> With this, remove the 'USE_THE_REPOSITORY_VARIABLE' macro from the file.

I suspect that this is not quite right.

    $ cd w/git.git/; make
    $ ./git-reflog list -h
    usage: git reflog list
    $ cd .. # not a repository
    $ git.git/git-reflog list -h
    fatal: not a git repository (or any of the parent directories): .git
    $ git.git/git-reflog -h
    usage: git reflog [show] ...

but I also suspect that it is mostly due to the original program
structure that uses OPT_SUBCOMMAND() that the subcommands fail to
respond to "-h" unlike the top-level command, so this may not be a
regression.  I do think however that this change is making it harder
to fix.

In any case, when you are adding a new feature, I would strongly
prefer you did *not* take it hostage to unrelated internal clean-up
with a dubious value.  For the library-ish parts of the system
(e.g. reflog.c at the top-level), not depending on the_repository is
absolutely the good thing to do, but the top level cmd_foo() are not
meant to be called as helper functions repeatedly with arbirary
repository instance, and a churn like this one, only to mollify the
USE_THE_REPOSITORY_VARIABLE macro, does not deserve to take a more
interesting (in the sense that it improves the life of end users)
change hostage by pretending to be its prerequisite clean-up.

Thanks.