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 .... 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 >>> >> >> >
