> Shouldn't evaluate_query_params(), evaluate_path_info(), and the
> subroutine that ties them together evaluate_and_validate_params() be
> in Gitweb::Request too?
>
>>
>> sub evaluate_git_dir {
> [....]
>> }
>
> Ditto with evaluate_git_dir()?
Well, evaluate_and_validate_params() and evaluate_path_info() contains
calls to subroutines which are not yet moved into any package. So,
what do you want to in such a case ?
> BTW. as I said in comment to previous patch, vriables such as $project
> should be put in Gitweb::Request in my opinion, not in Gitweb::Config.
> Perhaps they are, but it is not obvious from this patch.
>
>>
>> our (@snapshot_fmts, $git_avatar);
>> @@ -643,32 +605,32 @@ set_message(\&handle_errors_html);
>>
>> # dispatch
>> sub dispatch {
> [...]
>> }
>>
>> sub run_request {
>
> Those two subroutines should not, I guess, be put in Gitweb::Request.
> They would be in catch-all Gitweb module, I guess, or perhaps in the
> later post-GSoC future in Gitweb::Dispatch or something.
Yes.
>> @@ -689,7 +651,6 @@ sub run_request {
>> our $is_last_request = sub { 1 };
>> our ($pre_dispatch_hook, $post_dispatch_hook, $pre_listen_hook);
>> our $CGI = 'CGI';
>> -our $cgi;
>> sub evaluate_argv {
>> return unless (@ARGV);
>
> Ah, so $CGI is left in gitweb.perl, $cgi is moved to Gitweb::Request.
>
> [cut more than half of patch, which sould not be needed with exporting
> variables and not using fully qualified variable names]
>
>> diff --git a/gitweb/lib/Gitweb/Request.pm b/gitweb/lib/Gitweb/Request.pm
>> new file mode 100644
>> index 0000000..91cc492
>> --- /dev/null
>> +++ b/gitweb/lib/Gitweb/Request.pm
>> @@ -0,0 +1,58 @@
>> +#!/usr/bin/perl
>> +#
>> +# Gitweb::Request -- gitweb request(cgi) package
>> +#
>> +# This program is licensed under the GPLv2
>
> I don't remember what is git policy on copyright statements, and on
> GPLv2 vs GPLv2+...
>
>> +
>> +package Gitweb::Request;
>> +
>> +use strict;
>> +
>> +BEGIN {
>> + use Exporter();
>> +
>> + @Gitweb::Request::ISA = qw(Exporter);
>> + @Gitweb::Request::EXPORT = qw();
>> +}
>
> Same comment as for previous patch.
>
> use Exporter qw(import);
> our @EXPORT = qw($cgi, $my_url, $my_uri, $base_url, ...);
>
> BTW with exporting variables you can easily see here that
> Gitweb::Request does not use any variable from Gitweb::Config.
>
>> +
>> +our ($cgi, $my_url, $my_uri, $base_url, $path_info, $home_link);
>> +our ($action, $project, $file_name, $file_parent, $hash, $hash_parent, $hash_base,
>> + $hash_parent_base, @extra_options, $page, $git_dir);
>> +our ($searchtype, $search_use_regexp, $searchtext, $search_regexp);
>> +
>> +sub evaluate_uri {
> [...]
>> +}
>
> Straightforward code movement. But see comment above on which
> subroutines I feel should be also put in Gitweb::Request.
Please answer the question regarding calls to not-yet-packaged subroutines.
Thanks,
Pavan.
--
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