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

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

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(dot)zabuawala(at)enterprisedb(dot)com> wrote:

> Hi Dave,
>
> Please find attached PNG.
>
> On Thu, Apr 5, 2018 at 7:25 PM, Dave Page <dpage(at)pgadmin(dot)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 <
>> jdealmeidapereira(at)pivotal(dot)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 ....
>>>
>> ​(at)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(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
>>>>>>
>>>>>
>>>>>
>>>>
>>
>>
>> --
>> 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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2018-04-05 15:25:22 pgAdmin 4 commit: Allow sorting when viewing/editing data. Fixes #1894
Previous Message Murtuza Zabuawala 2018-04-05 14:03:02 Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable