Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hl93B-0004yv-Vj for pgadmin-hackers@arkaria.postgresql.org; Wed, 10 Jul 2019 09:38:10 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hl93A-0005hK-O4 for pgadmin-hackers@arkaria.postgresql.org; Wed, 10 Jul 2019 09:38:08 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hl93A-0005g9-Cm for pgadmin-hackers@lists.postgresql.org; Wed, 10 Jul 2019 09:38:08 +0000 Received: from mail-qt1-x835.google.com ([2607:f8b0:4864:20::835]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hl938-0000j3-0K for pgadmin-hackers@postgresql.org; Wed, 10 Jul 2019 09:38:07 +0000 Received: by mail-qt1-x835.google.com with SMTP id l9so1686405qtu.6 for ; Wed, 10 Jul 2019 02:38:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Jb9bTbUMdCpvsBCvr26z2tO8aifyv3xvMWOkgegGm/E=; b=FkGXqh8m5aI4SI+yKiC0Om4zIPr9rqYEEYpnRTg6j5U0yPJJDnWlysxXH5hsFsdEOg 5P898EFfjQ9efdy/I+8UEhVwDJ0+8N/5/ul7S3WnisJ1Dgqa/xLU7iBCjH7xmf8InWi3 5i3ogwt+8QhBrQ7ELwl0Yt+pyqg4oFmG0FxEzDvkoiKXUpqRM6f/nKwP/0Dy/1C1ibBX 93SQtApdQ6X0X86Hnj3u6MvwdGYCU765762xabeEGhi684ijgZKPOF3L4gBYfbjKAYfd 8oEJYy1OHfwDOLZX5MQz6RnMRPPig3itDJG2vFoyACWY6ALHJ3GTEJIXGAIRAa4Jj6Uw lotQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Jb9bTbUMdCpvsBCvr26z2tO8aifyv3xvMWOkgegGm/E=; b=SZ2Mj5SMDq14az0iWSwl2qpST8u5ZhZeasw4k6K9Q3sW1BUiA/UmqVICBEllzy6k/F qfPA5Oxa80JyJqN4GKwSxABxihqWVcOa1qnok22rqKSOpbl4I15vfD/jaqOiyhMdmuWM ALlQTDMNeFU5Kc9EWkPMR6SQYIhf4a9IHZhGpEyfnb2k49V/S+Xw4ai1ndNBHPD/jTGz YQmy27sZs1rCjcxf6VruwpFRcrJaScPEnKsITu2X2a2SBd7trZzjB8r8P8u4V1o6X2HU vUsJBmPIUQP1ca6aSy/li9+6E7wLWYo3CC66cub0T+zo85obHXC+HwEQJqIiiN3WqJlh szuA== X-Gm-Message-State: APjAAAVbcNlCgzkP/osBp4+Xv/eaukjxnz5F1/bvzQ1+e5UR7mfznsfI Qd2jze/O7rCGo4+t4nLFSwc6EHz+XGLerV2QlqM= X-Google-Smtp-Source: APXvYqx5NRwWGe5Bd0FMPiz7XI+nSRajyN/ZMu4XvGgYa1jOe+kmHv4HerotcWv5Oja8R9tb3aBTAn+Qx81xrAwwI2w= X-Received: by 2002:ad4:5283:: with SMTP id v3mr23856406qvr.207.1562751484432; Wed, 10 Jul 2019 02:38:04 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Yosry Muhammad Date: Wed, 10 Jul 2019 11:37:52 +0200 Message-ID: Subject: Re: [GSoC] Finalized First Patch To: Khushboo Vashi Cc: Dave Page , pgadmin-hackers@postgresql.org, Akshay Joshi Content-Type: multipart/alternative; boundary="0000000000001a15d9058d506f99" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0000000000001a15d9058d506f99 Content-Type: text/plain; charset="UTF-8" Hi Khushboo, On Wed, Jul 10, 2019, 8:33 AM Khushboo Vashi < khushboo.vashi@enterprisedb.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 ! > --0000000000001a15d9058d506f99 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Khushboo,

On Wed, Jul 10, 2019, 8:33 AM Khushboo Vas= hi <khushboo.vashi@en= terprisedb.com> wrote:
Hi Yosry,

I liked the= way you have refactored the code at some places in the JS file and made it= cleaner.

<= div dir=3D"auto">Thanks ! I am doing my best.

Here are some points:

1= . The table (including partition table) with a single column having that co= lumn primary key is editable but the save button is disabled, so, ultimatel= y I can't save the data. Note: The table should be empty to reproduce t= his issue.
2. command.py - The check_updatable_results_pkeys func= tion 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 sqledi= tor/__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 kn= ow.
3. In the Preferences, the label of the keyboard shortcut &qu= ot;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 k= eyboard shortcuts in the Preferences module as well as related to this feat= ure. Am I missing something?

I will look into those and g= et back to you asap. Thanks for the feedback !
--0000000000001a15d9058d506f99--