On 11/8/19 11:25 AM, Mark Dilger wrote:
>
>
> On 11/8/19 6:33 AM, Andrew Dunstan wrote:
>>
>> 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.
>
> I'd be happy to see the regression tests you are writing sooner than
> that, if you don't mind posting them. It's hard to do a proper review
> for you without a better sense of where you are going with these changes.
This will need to be rewritten in light of the above, but see
<https://www.postgresql.org/message-id/87b1e36b-e36a-add5-1a9b-9fa34914a256@2ndQuadrant.com>
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
