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
