Re: [PATCH v2 2/4] git-p4: create new method gitRunHook
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 5, 2020, 21:42 UTC
- Message-ID
- <xmqq36bohfsi.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <ac44531e-b02d-5a98-3e25-a305b1250cf6@gmail.com>
Ben Keene <seraphire@gmail.com> writes:
Show 9 quoted lines
>>> + hooks_path = os.path.join(os.environ.get("GIT_DIR", ".git"), "hooks")
>> This assumes that the process when his function is called (by the
>> way, even though the title of the patch uses the word "method", this
>> is not a method but a function, no?), it is always at the top level
>> of the working tree. Is that a good assumption? I don't know the
>> code well, so "yes it is good because a very early thing we do is to
>> go up to the top" is a good answer.
> I'm not sure what you mean by top level of the tree unless you mean
> that it is not part of a class, but a "Free standing" function?No. The discussion about function vs method was over immediately after we left the parentheses ;-)
The "top level of the working tree" is the directory where the files you see in "git ls-tree $commit^{tree}" would appear in. In our project, that is where the primary Makefile, COPYING, Documentation/, etc. hangs from.
The code in your patch (quoted above) says that "When $GIT_DIR is not set, '.git/hooks/' is the directory the hooks live in". That is true only when your process is at the top level of the working tree. If you chdir'ed to a subdirectory (e.g. Documentation/ in our project) and then let the quoted code run, hooks_path is set to ".git/hooks/", but from the point of view of the process running inside "Documentation/" subdirectory, it should actually be "../.git/hooks/", right?
> And > yes, it returns a value so it should be called a function. I'll > correct that.
This is an irrelevant tangent ;-) but yeah, it is a function, as opposed to a method, because it is not attached to any class. I did not think Python differentiated functions that return a value and ones that do not (e.g. Pascal call the latter "procedure").
> I chose to not put the function within a class so > that if other hooks should be added, it would not require a refactoring > of the code to use the function in other classes.
I think that is a sensible design decision to have a free-standing function to execute hooks.
Thanks.