Thanks, applied. On Fri, Apr 6, 2018 at 5:53 AM, Murtuza Zabuawala < murtuza.zabuaw...@enterprisedb.com> wrote:
> Hi, > > Please find updated patch which fixes PEP8 issue, added help string for > preference dialog option & updated screenshot. > > -- > Regards, > Murtuza Zabuawala > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > > On Thu, Apr 5, 2018 at 7:56 PM, Dave Page <dp...@pgadmin.org> wrote: > >> Sorry Murtuza - I'm mixing patches up here. It's the sort/filter one I >> need the image for please. >> >> Which means this patch needs re-creating as well please. >> >> On Thu, Apr 5, 2018 at 3:03 PM, Murtuza Zabuawala < >> murtuza.zabuaw...@enterprisedb.com> wrote: >> >>> Hi Dave, >>> >>> Please find attached PNG. >>> >>> On Thu, Apr 5, 2018 at 7:25 PM, Dave Page <dp...@pgadmin.org> wrote: >>> >>>> Murtuza, please send me the screenshot as a PNG. I've been tweaking the >>>> code and will recreate the patch with my changes. >>>> >>>> On Thu, Apr 5, 2018 at 2:53 PM, Joao De Almeida Pereira < >>>> jdealmeidapere...@pivotal.io> wrote: >>>> >>>>> Hi Murtuza, >>>>> Generally the patch looks good passes all CI but the linter fails: >>>>> >>>>> /tmp/build/4a5630c2/pivotal-rm-3072/web /tmp/build/4a5630c2 >>>>> >>>>> <https://gpdb-dev.bosh.pivotalci.info/teams/main/pipelines/pgadmin-feature-branches/jobs/pivotal-rm-3072-python-linter/builds/1#L5ab9877e:9> >>>>> ./pgadmin/browser/register_browser_preferences.py:11: [E302] expected 2 >>>>> blank lines, found 1 >>>>> >>>>> <https://gpdb-dev.bosh.pivotalci.info/teams/main/pipelines/pgadmin-feature-branches/jobs/pivotal-rm-3072-python-linter/builds/1#L5ab9877e:10> >>>>> 1 E302 expected 2 blank lines, found 1 >>>>> >>>>> <https://gpdb-dev.bosh.pivotalci.info/teams/main/pipelines/pgadmin-feature-branches/jobs/pivotal-rm-3072-python-linter/builds/1#L5ab9877e:11> >>>>> 1 >>>>> >>>>> >>>>> Did not test the functionality itself .... >>>>> >>>> ​@Joao, >>> I have included the test for Stats call which will cover the newly added >>> functionality.​ >>> >>> >>>> Thanks >>>>> Joao >>>>> >>>>> On Thu, Apr 5, 2018 at 8:09 AM Murtuza Zabuawala < >>>>> murtuza.zabuaw...@enterprisedb.com> wrote: >>>>> >>>>>> Hi Dave, >>>>>> >>>>>> Please find update patch. >>>>>> >>>>>> -- >>>>>> Regards, >>>>>> Murtuza Zabuawala >>>>>> EnterpriseDB: http://www.enterprisedb.com >>>>>> The Enterprise PostgreSQL Company >>>>>> >>>>>> >>>>>> On Thu, Apr 5, 2018 at 4:52 PM, Murtuza Zabuawala < >>>>>> murtuza.zabuaw...@enterprisedb.com> wrote: >>>>>> >>>>>>> On Thu, Apr 5, 2018 at 4:43 PM, Dave Page <dp...@pgadmin.org> wrote: >>>>>>> >>>>>>>> Hi >>>>>>>> >>>>>>>> On Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala < >>>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote: >>>>>>>> >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> PFA patch which allow user to configure how many rows they wish to >>>>>>>>> display for any pgagent jobs on statistics panel. >>>>>>>>> >>>>>>>> >>>>>>>> I think this is essentially good, however, I'm really not happy >>>>>>>> with the preference name and category. In general, I'd suggest that >>>>>>>> before >>>>>>>> creating patches in the future we should confirm naming etc on the >>>>>>>> mailing >>>>>>>> list, as I often end up changing wording and then requiring new >>>>>>>> screenshots >>>>>>>> etc. >>>>>>>> >>>>>>> ​Ok​ >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> In this case, I really don't like that we've added another >>>>>>>> category, and quite a specific one at that. I would suggest we move it >>>>>>>> to >>>>>>>> Browser -> Properties and name it "Maximum job history rows" with a >>>>>>>> description of "The maximum number of history rows to show on the >>>>>>>> Statistics tab for pgAgent jobs." >>>>>>>> >>>>>>> I'll change it and resend the patch. >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> Thoughts? >>>>>>>> >>>>>>>> -- >>>>>>>> Dave Page >>>>>>>> Blog: http://pgsnake.blogspot.com >>>>>>>> Twitter: @pgsnake >>>>>>>> >>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com >>>>>>>> The Enterprise PostgreSQL Company >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>> >>>> >>>> -- >>>> Dave Page >>>> Blog: http://pgsnake.blogspot.com >>>> Twitter: @pgsnake >>>> >>>> EnterpriseDB UK: http://www.enterprisedb.com >>>> The Enterprise PostgreSQL Company >>>> >>> >>> >> >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> > > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
