Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable - Mailing list pgadmin-hackers

From Joao De Almeida Pereira
Subject Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable
Date
Msg-id CAE+jjanp=CyyZ82-BhJmQSm0vVPaqJFJv=C3Noi115Oc3nLjGg@mail.gmail.com
Whole thread Raw
In response to Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable  (Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com>)
Responses Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable
List 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
./pgadmin/browser/register_browser_preferences.py:11: [E302] expected 2 blank lines, found 1
1 E302 expected 2 blank lines, found 1
1

Did not test the functionality itself ....

Thanks
Joao

On Thu, Apr 5, 2018 at 8:09 AM Murtuza Zabuawala <murtuza.zabuawala@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.zabuawala@enterprisedb.com> wrote:
On Thu, Apr 5, 2018 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <murtuza.zabuawala@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


pgadmin-hackers by date:

Previous
From: Joao De Almeida Pereira
Date:
Subject: Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree
Next
From: Dave Page
Date:
Subject: Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable