Re: [PATCH] Make is_gitfile a non-static generic function

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 11, 2011, 23:26 UTC
Message-ID: <7vsjmzcdpf.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4E94C8AB.3040807@cisco.com>

Phil Hord <hordp@cisco.com> writes:

> I'm not sure this function belongs in transport.c anymore, but
> I left it here to minimize conflicts.  I think a better home would
> be path.c, but maybe not.  If someone has a preference,
> please let me know.

I would think either transport.c or setup.c is more appropriate than path.c; the last one is more of "pathname manipulation utility bag of functions" and does not have much to do with the "Git"-ness of the path they deal with.

I am not sure if is_gitfile() is a good name for a global function, though. Also I think the interface to the function should be updated so that the caller can choose to receive the target path when the function returns with positive answer, making "read_gitfile()" unnecessary for such a caller.

As a matter of fact, couldn't we somehow unify these two slightly different implementations around the same theme, making is_gitfile() function unnecessary? As far as I can tell, the only difference between these functions is how they fail when given a non-gitfile, and many callers just call read_gitfile() without first asking if it is a gitfile.