On 11/8/19 1:16 AM, Craig Ringer wrote:
> On Fri, 8 Nov 2019 at 06:28, Mark Dilger <hornschnorter@gmail.com
> <mailto:hornschnorter@gmail.com>> wrote:
>
>
>
> On 10/31/19 10:02 AM, Andrew Dunstan wrote:
> >
> > This small patch authored by my colleague Craig Ringer enhances
> > Testlib's command_fails_like by allowing the passing of extra
> keyword
> > type arguments. The keyword initially recognized is
> 'extra_ipcrun_opts'.
> > The value for this keyword needs to be an array, and is passed
> through
> > to the call to IPC::Run.
>
> Hi Andrew, a few code review comments:
>
> The POD documentation for this function should be updated to
> include a
> description of the %kwargs argument list.
>
> Since command_fails_like is patterned on command_like, perhaps you
> should make this change to both of them, even if you only originally
> intend to use the new functionality in command_fails_like. I'm
> not sure
> of this, though, since I haven't seen any example usage yet.
>
> I'm vaguely bothered by having %kwargs gobble up the remaining
> function
> arguments, not because it isn't a perl-ish thing to do, but
> because none
> of the other functions in this module do anything similar. The
> function
> check_mode_recursive takes an optional $ignore_list array
> reference as
> its last argument. Perhaps command_fails_like could follow that
> pattern
> by taking an optional $kwargs hash reference.
>
>
> Yeah, that's probably sensible.
>
>
>
OK, I will rework it taking these comments into account. Thanks for the
comments Mark.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
