Looks good to me in general.  Below are some nitpicks (I'm probably
getting notorious for these).

[EMAIL PROTECTED] (Dave Rolsky) wrote:
>     print("\nFor testing purposes, please give the full path to an httpd\n",
>-        "with mod_perl enabled.  The path defaults to \$ENV{APACHE}, if present.");
>+        "with mod_perl enabled.  The path defaults to \$ENV{APACHE},\n",
>+        "if present (and that binary has mod_perl).");

I kind of feel like we should blindly use $ENV{APACHE} if it's there,
because I just made up the variable for this purpose, and if the user
sets it, we shouldn't argue.  It would be mysterious if you set APACHE
to something, and then Makefile.PL defaults to use something else
without telling you why.

But I don't feel very strongly about it.

What *would* be nice, though, is to check the user-typed response for
the presence of mod_perl, and give an appropriate error message if it's
missing.


>+sub _has_mod_perl {
>+    my ($self, $httpd) = @_;
>+
>+    foreach ( `$httpd -l` )  {
>+      return 1 if /mod_perl\.c/;
>+    }

Might as well just do:

  sub _has_mod_perl {
      my ($self, $httpd) = @_;

      return 1 if `$httpd -l` =~ /mod_perl\.c/;



  -------------------                            -------------------
  Ken Williams                             Last Bastion of Euclidity
  [EMAIL PROTECTED]                            The Math Forum

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to