Re: [PATCH/RFC] gitweb: Great subroutines renaming

From: Jakub Narebski <jnareb@gmail.com>
Date: Aug 8, 2006, 22:33 UTC
Message-ID: <ebb3fm$2md$1@sea.gmane.org>
In-Reply-To: <7vu04mucaq.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:

Show 7 quoted lines

>>  - git_blob_plain_mimetype => blob_plain_mimetype
> 
> Does it matter that the function is used in blob-plain?  In
> other words, how would this function, blob-plain-mimetype, be
> different if we were to have another function called blob-mimetype?
> 
> How about calling it "guess-mimetype"?

There are two functions, mimetype_guess_file and mimetype_guess, of which first one gets mimetype of file based on specified mime.types like file, second tries $mimetypes_file then '/etc/mime.types', while currently named git_blob_plain_mimetype subroutine first tries mimetype_guess, then checks if the file is text file (-T $fd) and if not uses some built in mime.types rules. Perhaps current mimetype_guess could be embedded into current git_blob_plain_mimetype.

All those subroutines need better names I think.

Show 5 quoted lines

>>  - git_page_nav => git_print_page_nav
>>  - git_header_div => git_print_header_div
> 
> Both sounds sane (I would have said "git-show-blah" if I were
> doing this myself, though).

They are called among print statements. I'm not sure if some of them
wouldn't be better converted to format_* types subroutines.

Show 10 quoted lines

>>  - read_info_ref => git_read_info_refs => git_get_references
>>  (this one depend too much on implementation, which might be changed to 
>>   parsing 'git ls-remotes .' output instead of relying on info/refs being
>>   up to date thanks to git-update-server-info in post-update hook, 
>>   and on its format).
> 
> I am not worried too much about the format (because clone/fetch
> over http depends on it), but reading from ls-remote self would
> make it unnecessary to run update-server-info if a repo is not
> served over http but is shown over gitweb.

Do the git_get_references for this subroutine, which returns hashref which keys are ids, and values are refs pointing to the key id, sounds good?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git