Re: RFC: ConfigDir option

> -----Original Message-----
> From: Orion Poplawski 
> Sent: Wednesday, November 30, 2016 18:24
> 
> 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

Um, typo.

--confdir

> 
> 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

Typo. --confdir

> > +$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.

I made that mistake in the beginning. The lesson take away here is plenty of documentation is required.


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot