Subject: Re: [PATCHv2 GSOC 10/11] gitweb: Create Gitweb::Format module

From: Jakub Narebski <[email-masked]>
Date: Sun, 18 Jul 2010 22:16:14 +0200

On Thu, 15 Jul 2010, Pavan Kumar Sunkara wrote:

> Create Gitweb::Format module in 'gitweb/lib/Gitweb/Format.pm'
> to store the subroutines related to formatting of HTML
> fragments required for gitweb.

What is the criterion of choosing whether to put given subroutine in
Gitweb::View / Gitweb::HTML and in Gitweb::Format (this module)?

> 
> This module depends on Config.pm, View.pm, Escape.pm,
> Util.pm and Request.pm.

O.K., although we should decide consistently through the series if we
want to use the Config.pm form for describing dependencies in the commit
message, or would it be better to use Gitweb::Config, i.e. the module
name.

>                         It mainly contain functions returning 
> short HTML fragments or transforming HTML fragments. Also
> include subroutines regarding avatar formatting.

I think you should say aomething about _why_ did you pur avatar
formatting in this module; I guess that is because git_get_avatar
returns also short HTML fragment.

> 
> Subroutines moved:
> 	format_log_line_html
> 	format_ref_marker
> 	format_subject_html
> 	picon_url
> 	gravatar_url
> 	git_get_avatar
> 	format_search_author
> 	format_author_html
> 	format_git_diff_header_line
> 	format_extended_diff_header_line
> 	format_diff_from_to_header
> 	format_diff_cc_simplified
> 	format_diff_line
> 	format_snapshot_links
> 
> Update 'gitweb/Makefile' to install Gitweb::Format alongside gitweb.
> 
> Signed-off-by: Pavan Kumar Sunkara <[email-masked]>
> ---

I have not checked patch itself.

-- 
Jakub Narebski
Poland