Re: [GSoC] Finalized First Patch

From: Yosry Muhammad <yosrym93(at)gmail(dot)com>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Cc: Dave Page <dpage(at)pgadmin(dot)org>, pgadmin-hackers(at)postgresql(dot)org, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Subject: Re: [GSoC] Finalized First Patch
Date: 2019-07-10 09:37:52
Message-ID: CAFSMqn-kBJ=MmGnEKFk7bdqZkPaoLOHJkVv-w+iFDdUO3Bs6SQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Khushboo,

On Wed, Jul 10, 2019, 8:33 AM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

> Hi Yosry,
>
> I liked the way you have refactored the code at some places in the JS file
> and made it cleaner.
>

Thanks ! I am doing my best.

Here are some points:
>
> 1. The table (including partition table) with a single column having that
> column primary key is editable but the save button is disabled, so,
> ultimately I can't save the data. Note: The table should be empty to
> reproduce this issue.
> 2. command.py - The check_updatable_results_pkeys function calling the
> poll function and checks the ASYNC_OK, I think this is not required as this
> function is called from the poll function from the sqleditor/__init__.py
> *if the status of the polling is if ASYNC_OK*. So, I think this is overhead
> but if you have considered another scenario then let me know.
> 3. In the Preferences, the label of the keyboard shortcut "Save Data
> Changes" should be "Save data changes".
> 4. Dave has already mentioned about the commented code, so I do agree we
> should remove it.
> 5. I didn't find the doc updates for the keyboard shortcuts in the
> Preferences module as well as related to this feature. Am I missing
> something?
>

I will look into those and get back to you asap. Thanks for the feedback !

>

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Yosry Muhammad 2019-07-10 09:41:13 Re: [GSoC] Finalized First Patch
Previous Message Khushboo Vashi 2019-07-10 07:14:07 Re: [GSoC] Finalized First Patch