From: Jakub Narebski <jnareb@gmail.com>
To: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>
Cc: git@vger.kernel.org, Christian Couder <chriscool@tuxfamily.org>,
Petr Baudis <pasky@ucw.cz>
Subject: Re: [PATCHv2 GSOC 08/11] gitweb: Create Gitweb::View module
Date: Sun, 18 Jul 2010 17:10:22 +0200 [thread overview]
Message-ID: <201007181710.23105.jnareb@gmail.com> (raw)
In-Reply-To: <1279178951-23712-9-git-send-email-pavan.sss1991@gmail.com>
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@gmail.com>
I have not checked the diff itself...
--
Jakub Narebski
Poland
next prev parent reply other threads:[~2010-07-18 15:10 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-15 7:29 [PATCHv2 00/11] Splitting gitweb Pavan Kumar Sunkara
2010-07-15 7:29 ` [PATCHv2 GSOC 01/11] gitweb: fix esc_url Pavan Kumar Sunkara
2010-07-15 13:52 ` Jakub Narebski
2010-07-15 18:57 ` Junio C Hamano
2010-07-15 19:32 ` Jakub Narebski
2010-07-15 7:29 ` [PATCHv2 GSOC 02/11] gitweb: Prepare for splitting gitweb Pavan Kumar Sunkara
2010-07-15 18:05 ` Jakub Narebski
2010-07-15 7:29 ` [PATCHv2 GSOC 03/11] gitweb: Create Gitweb::Git module Pavan Kumar Sunkara
2010-07-15 20:13 ` Jakub Narebski
2010-07-15 7:29 ` [PATCHv2 GSOC 04/11] gitweb: Create Gitweb::Config module Pavan Kumar Sunkara
2010-07-15 21:21 ` Jakub Narebski
2010-07-15 7:29 ` [PATCHv2 GSOC 05/11] gitweb: Create Gitweb::Request module Pavan Kumar Sunkara
2010-07-16 0:11 ` Jakub Narebski
2010-07-15 7:29 ` [PATCHv2 GSOC 06/11] gitweb: Create Gitweb::Escape module Pavan Kumar Sunkara
2010-07-16 9:01 ` Jakub Narebski
2010-07-15 7:29 ` [PATCHv2 GSOC 07/11] gitweb: Create Gitweb::RepoConfig module Pavan Kumar Sunkara
2010-07-16 12:11 ` Jakub Narebski
2010-07-15 7:29 ` [PATCHv2 GSOC 08/11] gitweb: Create Gitweb::View module Pavan Kumar Sunkara
2010-07-18 15:10 ` Jakub Narebski [this message]
2010-07-15 7:29 ` [PATCHv2 GSOC 09/11] gitweb: Create Gitweb::Util module Pavan Kumar Sunkara
2010-07-18 17:45 ` Jakub Narebski
2010-07-15 7:29 ` [PATCHv2 GSOC 10/11] gitweb: Create Gitweb::Format module Pavan Kumar Sunkara
2010-07-18 20:16 ` Jakub Narebski
2010-07-15 7:29 ` [PATCHv2 GSOC 11/11] gitweb: Create Gitweb::Parse module Pavan Kumar Sunkara
2010-07-19 14:55 ` Jakub Narebski
2010-08-01 20:44 ` [PATCHv2 00/11] Splitting gitweb Sverre Rabbelier
2010-08-02 15:03 ` Jakub Narebski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201007181710.23105.jnareb@gmail.com \
--to=jnareb@gmail.com \
--cc=chriscool@tuxfamily.org \
--cc=git@vger.kernel.org \
--cc=pasky@ucw.cz \
--cc=pavan.sss1991@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.