Thanks, patch applied.
On Mon, Apr 10, 2017 at 2:22 PM, Navnath Gadakh
<navnath.gadakh@enterprisedb.com> wrote:
> Hi Dave,
>
> Please find the revised patch.
>
> On Mon, Apr 10, 2017 at 1:43 PM, Dave Page <dave.page@enterprisedb.com>
> wrote:
>>
>> Hi
>>
>> On Fri, Apr 7, 2017 at 8:01 PM, Navnath Gadakh
>> <navnath.gadakh@enterprisedb.com> wrote:
>>>
>>> Hello Joao,
>>>
>>> Thanks for review and suggestions.
>>>
>>> On Fri, Apr 7, 2017 at 9:08 PM, Joao Pedro De Almeida Pereira
>>> <jdealmeidapereira@pivotal.io> wrote:
>>>>
>>>> Hello Hackers,
>>>> We just looked into this patch and have some questions about it.
>>>>
>>>> - Where is the JSON file created? Do we need to call it with some
>>>> special argument in order for it to be created?
>>>
>>> This file is created under /regression/ directory (runtests.py, line
>>> 401). No need to call any special argument.
>>>>
>>>> - There is a print statement in the line 275 of runtests.py is it
>>>> supposed to be there?
>>>
>>> Removed.
>>>>
>>>> - The function name addSuccess does not match the styling of the code,
>>>> it should be add_success
>>>
>>> Done.
>>>>
>>>> Suggestions:
>>>> - The definition of the class_name (run_tests.py, line 229) variable
>>>> looks the same as in the if statements below and could be extracted into a
>>>> function to avoid repeating the same code.
>>>
>>> Done.
>>>>
>>>> - In the same function when we are updating error/failure/skip test
>>>> results the code looks pretty similar and can also be extracted into a
>>>> function
>>>
>>> Done.
>>>
>>>
>>> @Dave, Please find the attached patch with necessary code changes. I have
>>> also added missing scenario names to some test cases.
>>
>>
>> The passed test results are shown as null. They should either be removed
>> (because they'll all be "Passed" or similar anyway), or set to "Passed" or
>> "Pass".
>
> Ok. For consistency purposed with other test case type I have set it to
> ''Passed".
>
> Also, did some code cleanup in the attached patch.
>
> Thanks.
>>
>>
>> Thanks.
>>
>> --
>> Dave Page
>> VP, Chief Architect, Tools & Installers
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>
>
>
>
> --
> Regards,
> Navnath Gadakh
>
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company
>
>
--
Dave Page
VP, Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake