Re: [bug fix] pg_ctl always uses the same event source

On Fri, Jul 18, 2014 at 7:08 PM, MauMau <maumau307(at)gmail(dot)com> wrote:
>
> From: "Magnus Hagander" <magnus(at)hagander(dot)net>
>
>> On Fri, Jul 18, 2014 at 5:33 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:
>>>
>>> On Thu, Jul 17, 2014 at 4:51 PM, Magnus Hagander <magnus(at)hagander(dot)net>
>>> wrote:
>>>>
>>>>
>>>> Did anyone actually test this patch? :)
>>>>
>>>> I admit I did not build it on Windows specifically because I assumed
>>>> that was done as part of the development and review. And the changes
>>>> to pg_event.c can never have built, since the file does not include
>>>> the required header.
>>>
>>>
>>> I have tested it on Windows and infact on Linux as well to see if there
>>> is any side impact before marking it as Ready For Committer.
>>>
>>> It seems to me that the required header is removed in last version
>>> (pg_ctl_eventsrc_v11) where MessageBox() related changes have been
>>> removed from patch as per recent discussion. Sorry for not being able
>>> to check last version posted.
>>
>>
>> Gotcha. Thanks for clarifying, and I apologize if I came across a bit
>> harsh even with the smiley.

The statement was not at all harsh. I think you are right in asking that
question and I believe that is the minimum expectation once the patch
reaches Ready To Committer stage.

>
> I'm sorry to have caused both of you trouble. I have to admit that I
didn't compile the source when I removed the MessageBox()-related changes.
The attached patch fixes that. I confirmed successful build this time.

I have tested the attached patch on windows, it works fine both for
default and non-default cases. It passes other regression tests as well
and build went fine on Linux.

One more thing about inclusion of new header file in pgevent.c, I think
that is okay because we include it in other modules (client side) present
in bin directory like reindex.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com