On Thu, 15 Jul 2010, Pavan Kumar Sunkara wrote: > Create Gitweb::View module in 'gitweb/lib/Gitweb/View.pm' > to store the subroutines related to the HTML output > for gitweb. If it contains subroutines related only to HTML output, why isn't it called Gitweb::HTML then? If it contains some subroutines which are not strictly about HTML, please state it in the commit message. > > This module depends on Git.pm, Config.pm, Request.pm, > Escape.pm and RepoConfig.pm. Action specific HTML div > subroutines are not included in this module due to > unmet dependencies of Gitweb::Parse and Gitweb::Format. Whether you use Gitweb::Git or Git.pm, you should be consistent in the naming you use: either Git.pm, or Gitweb::Parse. Do I understand correctly trhat Gitweb::Parse and Gitweb::Format are to be added in subsequent patches? Please state this fact in the commit message. > > Subroutines moved: > *href > get_feed_info > *chop_str > chop_and_escape_str > *age_class > age_string > *S_ISGITLINK > mode_str > file_type > file_type_long > *get_page_title > git_header_html > git_footer_html > die_error > *git_print_page_nav > format_paging_nav > git_print_header_div > git_print_page_path > *print_local_time > format_local_time > *insert_file > *git_get_link_target > normalize_link_target > git_print_tree_entry > *print_sort_th > format_sort_th Nitpick: my first thought was "why some of those subroutines are marked with a star?". If you want to divide this list of subroutines into groups by function, you should leave a space between '*' and name of subroutine (i.e. '* href'), and perhaps also leave empty lines between groups, though this might make commit this list overly long. I guess that this separation of subroutines into Gitweb::View module was a matter of some compromise: between having too large modules (containing only barely related subroutines) and having too many modules. > > Update 'gitweb/Makefile' to install Gitweb::View alongside gitweb. > > Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@xxxxxxxxx> I have not checked the diff itself... -- Jakub Narebski Poland -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html