Обсуждение: [pgadmin-hackers] [patch] upgrade slickgrid

Поиск
Список
Период
Сортировка

[pgadmin-hackers] [patch] upgrade slickgrid

От
Joao Pedro De Almeida Pereira
Дата:
Hello Hackers

We upgraded SlickGrid to the latest version + 53ee34. We upgraded since we are working on some changes to the query results functionality, including improvements to column selection and shortcuts.

This SlickGrid upgrade includes a PR we wrote that simplifies some of these changes.

Please note that we are not requesting this be included in the 1.5 release.

Thanks!
Joao and George
Вложения

Re: [pgadmin-hackers] [patch] upgrade slickgrid

От
Merlin Moncure
Дата:
On Mon, May 15, 2017 at 1:35 PM, Joao Pedro De Almeida Pereira
<jdealmeidapereira@pivotal.io> wrote:
> Hello Hackers
>
> We upgraded SlickGrid to the latest version + 53ee34. We upgraded since we
> are working on some changes to the query results functionality, including
> improvements to column selection and shortcuts.
>
> This SlickGrid upgrade includes a PR we wrote that simplifies some of these
> changes.
>
> Please note that we are not requesting this be included in the 1.5 release.
>
> Thanks!
> Joao and George

hm, curious if you've moved off the mostly abandoned main fork
(https://github.com/mleibman/SlickGrid) to the currently active one
(https://github.com/6pac/SlickGrid)...

merlin

Re: [pgadmin-hackers] [patch] upgrade slickgrid

От
Matthew Kleiman
Дата:
Hey Merlin

Yes, we are using the 6pac fork. We've found Ben (6pac) to be very responsive. He accepted two PRs we wrote to the SlickGrid in a very timely manner. 

Thanks,
Matt

On Mon, May 15, 2017 at 4:46 PM Merlin Moncure <mmoncure@gmail.com> wrote:
On Mon, May 15, 2017 at 1:35 PM, Joao Pedro De Almeida Pereira
<jdealmeidapereira@pivotal.io> wrote:
> Hello Hackers
>
> We upgraded SlickGrid to the latest version + 53ee34. We upgraded since we
> are working on some changes to the query results functionality, including
> improvements to column selection and shortcuts.
>
> This SlickGrid upgrade includes a PR we wrote that simplifies some of these
> changes.
>
> Please note that we are not requesting this be included in the 1.5 release.
>
> Thanks!
> Joao and George

hm, curious if you've moved off the mostly abandoned main fork
(https://github.com/mleibman/SlickGrid) to the currently active one
(https://github.com/6pac/SlickGrid)...

merlin


--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [patch] upgrade slickgrid

От
Dave Page
Дата:
Hi

On Mon, May 15, 2017 at 7:35 PM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello Hackers

We upgraded SlickGrid to the latest version + 53ee34. We upgraded since we are working on some changes to the query results functionality, including improvements to column selection and shortcuts.

This SlickGrid upgrade includes a PR we wrote that simplifies some of these changes.

Please note that we are not requesting this be included in the 1.5 release.

Have you confirmed that removing the changes we made to shorten the class names used for rows and cells doesn't affect the performance or memory utilisation of the grid? 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgadmin-hackers] [patch] upgrade slickgrid

От
Dave Page
Дата:


On Wed, May 17, 2017 at 1:59 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Mon, May 15, 2017 at 7:35 PM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello Hackers

We upgraded SlickGrid to the latest version + 53ee34. We upgraded since we are working on some changes to the query results functionality, including improvements to column selection and shortcuts.

This SlickGrid upgrade includes a PR we wrote that simplifies some of these changes.

Please note that we are not requesting this be included in the 1.5 release.

Have you confirmed that removing the changes we made to shorten the class names used for rows and cells doesn't affect the performance or memory utilisation of the grid? 

Also... would this be an appropriate time to remove the embedded libraries altogether, and move to npm/yarn installations of them? 


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgadmin-hackers] [patch] upgrade slickgrid

От
Matthew Kleiman
Дата:
Hi Dave

Have you confirmed that removing the changes we made to shorten the class names used for rows and cells doesn't affect the performance or memory utilisation of the grid? 

We haven't noticed any performance issues with this upgraded version of SlickGrid, nor our other changes to the query results grid on the branch that we will be submitting soon. Do you have an example of a large table that used to cause you performance problems? 

Also... would this be an appropriate time to remove the embedded libraries altogether, and move to npm/yarn installations of them? 

I think that would be a great idea. Moving the vendor-ed libraries into a package manager would help us manage upgrading of libraries. The package.json file is a more standard format for managing javascript dependencies, when compared to our libraries.txt file.

Regards,
Matt

On Wed, May 17, 2017 at 9:04 AM Dave Page <dpage@pgadmin.org> wrote:
On Wed, May 17, 2017 at 1:59 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Mon, May 15, 2017 at 7:35 PM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello Hackers

We upgraded SlickGrid to the latest version + 53ee34. We upgraded since we are working on some changes to the query results functionality, including improvements to column selection and shortcuts.

This SlickGrid upgrade includes a PR we wrote that simplifies some of these changes.

Please note that we are not requesting this be included in the 1.5 release.

Have you confirmed that removing the changes we made to shorten the class names used for rows and cells doesn't affect the performance or memory utilisation of the grid? 

Also... would this be an appropriate time to remove the embedded libraries altogether, and move to npm/yarn installations of them? 


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgadmin-hackers] [patch] upgrade slickgrid

От
Dave Page
Дата:
Hi

On Wed, May 17, 2017 at 4:45 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:
Hi Dave

Have you confirmed that removing the changes we made to shorten the class names used for rows and cells doesn't affect the performance or memory utilisation of the grid? 

We haven't noticed any performance issues with this upgraded version of SlickGrid, nor our other changes to the query results grid on the branch that we will be submitting soon. Do you have an example of a large table that used to cause you performance problems? 

Nothing specific - large tables are easily generated though. Mostly what we had were numerous reports from users of poor performance in the query tool, and this was one of a number of changes we've made (and are continuing to make) to improve things. This particular change was made on the basis of logic and math though, not testing of it on it's own. It's possible it's a non-issue if, for example, SlickGrid is only actually keeping a small fixed number of rows in the DOM at any one time. Either way, I'd like to confirm the change won't cause a performance regression before applying.
 

Also... would this be an appropriate time to remove the embedded libraries altogether, and move to npm/yarn installations of them? 

I think that would be a great idea. Moving the vendor-ed libraries into a package manager would help us manage upgrading of libraries. The package.json file is a more standard format for managing javascript dependencies, when compared to our libraries.txt file.

Feel like putting that in your backlog? :-) 


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgadmin-hackers] [patch] upgrade slickgrid

От
Robert Eckhardt
Дата:


On Thu, May 18, 2017 at 4:42 AM, Dave Page <dpage@pgadmin.org> wrote:


Feel like putting that in your backlog? :-) 

Interesting you say that. It is either going to be trivial to do or we won't have the context on which vendored libraries have been changed. We have a task to try and see if it is trivial. If it is we will submit a patch shortly. If it isn't trivial we will probably start a conversation on the mailing list with a list of tests that failed because of vendored libraries. 

-- Rob

Re: [pgadmin-hackers] [patch] upgrade slickgrid

От
Dave Page
Дата:


On Thu, May 18, 2017 at 2:04 PM, Robert Eckhardt <reckhardt@pivotal.io> wrote:


On Thu, May 18, 2017 at 4:42 AM, Dave Page <dpage@pgadmin.org> wrote:


Feel like putting that in your backlog? :-) 

Interesting you say that. It is either going to be trivial to do or we won't have the context on which vendored libraries have been changed. We have a task to try and see if it is trivial. If it is we will submit a patch shortly. If it isn't trivial we will probably start a conversation on the mailing list with a list of tests that failed because of vendored libraries. 

Any that we have changed will have a README added to them explaining what was done and why. I think it's only wcDocker and SlickGrid off the top of my head. 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company