Re: RFC: ConfigDir option

On 11/28/2016 05:27 PM, Jason Pyeron wrote:
> Request for comments, concerns, objections, suggestions.
> 
> Background:
> 
> While developing a testing framework, we needed to support a runtime configuration for the loading of a configuration.
> 
> Solution:
> 
> --configdir option

Seems mostly reasonable to me.  Some comments below.

> Implementation:
> 
> root@black7 /projects/logwatch
> $ git diff
> diff --git a/scripts/logwatch.pl b/scripts/logwatch.pl
> index 0167755..fd5dd6a 100755
> --- a/scripts/logwatch.pl
> +++ b/scripts/logwatch.pl
> @@ -62,6 +62,11 @@ use File::Temp qw/ tempdir /;
>  eval "use lib \"$BaseDir/lib\";";
>  eval "use Logwatch \':dates\'";
> 
> #The current placement of the GetOptions is well after the contents of /etc/logwatch/ are ingested. So a preliminary call is added before:
> # http://search.cpan.org/dist/Getopt-Long/lib/Getopt/Long.pm 
> #Since Getopt::Long modifies ARGV, then we clone the array.
> +my @argvClone = @ARGV;

We're not particularly consistent, but I'd prefer "ArgvClone".

> #Most usage of Getopt::Long is based on a global context and the configuration would impact the second call later on, so lets use a OO desgin and instantiate an instance.
> +my $gop = Getopt::Long::Parser->new;

gop -> GetOptParser

> #ignore the unknown options, we only care about --configdir
> +$gop->configure("pass_through");
> #extract confdir, if it exists
> +$gop->getoptionsfromarray(\@argvClone, "confdir=s" => \$ConfigDir);

Above you mention "configdir", but here it's "confdir".  Not sure I have a
preference.  It's conceivable that someone might thing --confdir would point
to /etc/logwatch/conf, but probably unlikely.

-- 
Orion Poplawski
Technical Manager                     303-415-9701 x222
NWRA, Boulder/CoRA Office             FAX: 303-415-9702
3380 Mitchell Lane                       [email protected]
Boulder, CO 80301                   http://www.nwra.com

------------------------------------------------------------------------------