Обсуждение: Bug #3083 fix
Вложения
Hi
--
On Fri, Mar 2, 2018 at 3:40 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello Neethu,We passed the patch through our CI pipeline and all tests pass.The code looks good, but we are trying to decouple files as much as we can so that we do not end up with files with over 1000 lines, that are hard to read and to maintain. Also we are trying to create Unit Tests to have more coverage in our Javascript code.Can you split the new implementation code into it's own file and create some tests to ensure the behavior will not be broken in the future?iYou have some examples on: pgadmin/browser/server_groups/servers/databases/ external_tables/*
I have spilt the new implementation into different file. Its' been hard to write jasmine/feature test case as it requires drag event and exact co-ordinate to resize the slickgrid cell.
Attached is the modified patch.
ThanksJoaoOn Thu, Mar 1, 2018 at 10:37 AM Neethu Mariya Joy <neethumariyajoy@gmail.com> wrote:Hi,I am Neethu Mariya Joy, an undergraduate pursuing BE in Computer Science at BITS Pilani.I've attempted to fix https://redmine.postgresql.org/issues/3083. Since the textarea resize feature is the default HTML feature, I have not changed it. Instead, I've added draggable borders to the wrapper which expands the textarea inside it. I'm attaching my patch as bug3083.diff below as per the contribution guidelines.Hope this helps. Thank you for your consideration!Sincerely,Neethu Mariya Joy
Akshay Joshi
Sr. Software Architect
Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
Mobile: +91 976-788-8246
Вложения
Hey Akshay and Neethu
We refactored the patch to add tests for the resize feature. We were able to write test cases for the drag event by using spies and setting the rect dimensions. In cases like this, we can just test some components in order to have enough confidence in the code. So we isolated the function that implements the behavior of this feature and tested that it was performing as expected.
We ran the patch through the pipelines and all of the tests passed.
Sincerely,
Joao and Victoria
On Wed, Mar 28, 2018 at 8:03 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
HiOn Fri, Mar 2, 2018 at 3:40 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:Hello Neethu,We passed the patch through our CI pipeline and all tests pass.The code looks good, but we are trying to decouple files as much as we can so that we do not end up with files with over 1000 lines, that are hard to read and to maintain. Also we are trying to create Unit Tests to have more coverage in our Javascript code.Can you split the new implementation code into it's own file and create some tests to ensure the behavior will not be broken in the future?iYou have some examples on: pgadmin/browser/server_groups/servers/databases/external_tables/*I have spilt the new implementation into different file. Its' been hard to write jasmine/feature test case as it requires drag event and exact co-ordinate to resize the slickgrid cell.Attached is the modified patch.ThanksJoaoOn Thu, Mar 1, 2018 at 10:37 AM Neethu Mariya Joy <neethumariyajoy@gmail.com> wrote:Hi,I am Neethu Mariya Joy, an undergraduate pursuing BE in Computer Science at BITS Pilani.I've attempted to fix https://redmine.postgresql.org/issues/3083. Since the textarea resize feature is the default HTML feature, I have not changed it. Instead, I've added draggable borders to the wrapper which expands the textarea inside it.I'm attaching my patch as bug3083.diff below as per the contribution guidelines.Hope this helps. Thank you for your consideration!Sincerely,Neethu Mariya Joy
Вложения
Thanks Joao.
--
On Wed, Mar 28, 2018 at 11:36 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hey Akshay and NeethuWe refactored the patch to add tests for the resize feature. We were able to write test cases for the drag event by using spies and setting the rect dimensions. In cases like this, we can just test some components in order to have enough confidence in the code. So we isolated the function that implements the behavior of this feature and tested that it was performing as expected.We ran the patch through the pipelines and all of the tests passed.Sincerely,Joao and VictoriaHiOn Fri, Mar 2, 2018 at 3:40 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:Hello Neethu,We passed the patch through our CI pipeline and all tests pass.The code looks good, but we are trying to decouple files as much as we can so that we do not end up with files with over 1000 lines, that are hard to read and to maintain. Also we are trying to create Unit Tests to have more coverage in our Javascript code.Can you split the new implementation code into it's own file and create some tests to ensure the behavior will not be broken in the future?iYou have some examples on: pgadmin/browser/server_groups/servers/databases/ external_tables/* I have spilt the new implementation into different file. Its' been hard to write jasmine/feature test case as it requires drag event and exact co-ordinate to resize the slickgrid cell.Attached is the modified patch.ThanksJoaoOn Thu, Mar 1, 2018 at 10:37 AM Neethu Mariya Joy <neethumariyajoy@gmail.com> wrote:Hi,I am Neethu Mariya Joy, an undergraduate pursuing BE in Computer Science at BITS Pilani.I've attempted to fix https://redmine.postgresql.org/issues/3083. Since the textarea resize feature is the default HTML feature, I have not changed it. Instead, I've added draggable borders to the wrapper which expands the textarea inside it. I'm attaching my patch as bug3083.diff below as per the contribution guidelines.Hope this helps. Thank you for your consideration!Sincerely,Neethu Mariya Joy
Akshay Joshi
Sr. Software Architect
Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
Mobile: +91 976-788-8246