Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable

From: Joao De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
Cc: Dave Page <dpage(at)pgadmin(dot)org>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable
Date: 2018-04-05 13:53:18
Message-ID: CAE+jjanp=CyyZ82-BhJmQSm0vVPaqJFJv=C3Noi115Oc3nLjGg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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(dot)zabuawala(at)enterprisedb(dot)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(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>
>> On Thu, Apr 5, 2018 at 4:43 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Hi
>>>
>>> On Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <
>>> murtuza(dot)zabuawala(at)enterprisedb(dot)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
>>>
>>
>>
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2018-04-05 13:55:08 Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable
Previous Message Joao De Almeida Pereira 2018-04-05 13:38:31 Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree