Thanks, patch applied. On Mon, Apr 10, 2017 at 2:22 PM, Navnath Gadakh <navnath.gad...@enterprisedb.com> wrote: > Hi Dave, > > Please find the revised patch. > > On Mon, Apr 10, 2017 at 1:43 PM, Dave Page <dave.p...@enterprisedb.com> > wrote: >> >> Hi >> >> On Fri, Apr 7, 2017 at 8:01 PM, Navnath Gadakh >> <navnath.gad...@enterprisedb.com> wrote: >>> >>> Hello Joao, >>> >>> Thanks for review and suggestions. >>> >>> On Fri, Apr 7, 2017 at 9:08 PM, Joao Pedro De Almeida Pereira >>> <jdealmeidapere...@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 -- Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers
