public inbox for [email protected]
help / color / mirror / Atom feedFrom: Yosry Muhammad <[email protected]>
To: Khushboo Vashi <[email protected]>
Cc: Dave Page <[email protected]>
Cc: [email protected]
Cc: Akshay Joshi <[email protected]>
Subject: Re: [GSoC] Finalized First Patch
Date: Wed, 10 Jul 2019 11:37:52 +0200
Message-ID: <CAFSMqn-kBJ=MmGnEKFk7bdqZkPaoLOHJkVv-w+iFDdUO3Bs6SQ@mail.gmail.com> (raw)
In-Reply-To: <CAFOhELeFLVLS_RDTJPH0cLDRP8PUxFfxL05YXcL0+r1addP8dg@mail.gmail.com>
References: <CAFSMqn8yLA329GFHSm0Bn4cEqvd+kVoYpCAB5mRDbVF6NNaxxA@mail.gmail.com>
<CAFSMqn8fjFT8CbhB5FAtkN8cd7QH0+XYtSUZmD5J1F38n1189g@mail.gmail.com>
<CA+OCxoz=50F2N6+7dGrarieXNaXiiJw-deSAXd6fSH7SvEaQXw@mail.gmail.com>
<CAFSMqn9HaapnutknGQdG2h0-xyk9qCcGB_wxkPM5iwiHp=fPpg@mail.gmail.com>
<CA+OCxox43r3qH9YNt_gT7TT3s+DkROvf1pvvAKzg663xHzT5VQ@mail.gmail.com>
<CAFOhELeFLVLS_RDTJPH0cLDRP8PUxFfxL05YXcL0+r1addP8dg@mail.gmail.com>
Hi Khushboo,
On Wed, Jul 10, 2019, 8:33 AM Khushboo Vashi <
[email protected]> 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 !
>
view thread (19+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: [GSoC] Finalized First Patch
In-Reply-To: <CAFSMqn-kBJ=MmGnEKFk7bdqZkPaoLOHJkVv-w+iFDdUO3Bs6SQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox