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 <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 22:40:25 |
Message-ID: | CAFSMqn9joAdZzpsbxZXMOSW5jYOsXM-mx8nj5=eUPRokYpXtYA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
Hi,
Please find an updated patch attached.
On Wed, Jul 10, 2019 at 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.
> 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.
>
I tried to reproduce the issue but it works fine for me. Please make sure
that you edit the empty cell (to add a new row) and press enter for the
edits on the grid to take effect.
> 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.
>
It was just a sanity check, just in case someone calls the function
incorrectly. I removed it and added comments below the function header
indicating when it should be called.
> 3. In the Preferences, the label of the keyboard shortcut "Save Data
> Changes" should be "Save data changes".
>
Corrected.
> 4. Dave has already mentioned about the commented code, so I do agree we
> should remove it.
>
Removed.
> 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 don't know which part exactly are you referring to.
In the previous patch, I have already updated keyboard_shortcuts.rst to
document the new button shortcut, in addition to preferences.rst to
document the new 'Alert on uncommited changes' option. I also updated
query_tool.rst, query_tool_toolbar.rst & editgrid.rst to document the new
features. Which part is missing?
Looking forward to your feedback and comments.
Thanks !
Attachment | Content-Type | Size |
---|---|---|
query_tool_automatic_mode_switch_v1.8.patch | text/x-patch | 480.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Khushboo Vashi | 2019-07-11 04:36:17 | Re: [GSoC] Finalized First Patch |
Previous Message | navnath gadakh | 2019-07-10 14:54:57 | RE-SQL tests patch for packages node |