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 1hkhkd-0007mX-L0 for pgadmin-hackers@arkaria.postgresql.org; Tue, 09 Jul 2019 04:29:12 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hkhkZ-0000Cy-SD for pgadmin-hackers@arkaria.postgresql.org; Tue, 09 Jul 2019 04:29:07 +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 1hkhkZ-0000Cd-9d for pgadmin-hackers@lists.postgresql.org; Tue, 09 Jul 2019 04:29:07 +0000 Received: from mail-ot1-x343.google.com ([2607:f8b0:4864:20::343]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hkhkR-0000Kt-H6 for pgadmin-hackers@postgresql.org; Tue, 09 Jul 2019 04:29:05 +0000 Received: by mail-ot1-x343.google.com with SMTP id e8so18543261otl.7 for ; Mon, 08 Jul 2019 21:28:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=YVuJfY9nO93K8ZDorH+VjKqSGW0Qk+ykMirkdCcVd/8=; b=ccXD/ecT1zwRBuo+e10znvAEnGoQmoP6iVWC5ksHNVVuRLIMlyne2x5vcUAzfZglwu tZ91jYaUK6TdVR+OYX+oceY+9gqTIAWTOhDYIeGbMLU/njCgA1NJ6844pbpPBNtrxPeL 5+e4aAfIy0/WISQ20tR8Ir4XNz5dR++TPUV5FuNrHVxTKyLDncsIBUqShhLNQagu+PMn 3HuOzg6DXwPNIzluyCOSWWCVUjPiLme1affvznHmPzUUUamW/za4f2CF5iMdQS5+Wzos 6TlypwQAp8d32l6QR2MzSeezF7/O553q3pSf++d/VZPt7LMF2wrCOhdm94P36wsaW+Ji 9q8w== 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=YVuJfY9nO93K8ZDorH+VjKqSGW0Qk+ykMirkdCcVd/8=; b=MYKSGSEi4XJVXD9llM5He2Ika4nZ2yRql6L6zACI9HCpfW0aCjopXMogHI9SvtGM0n zENrI2/2dPZfhDPJw2ZI3keXRB1kzA/VC5fyPCf8mJiAHcrcmrTbCrxjmOKzwofA44Jf Q8ixLMOrIAh3Hs7HVIjQIrbXLzyGMILMZd9K6/OhS7zvc9LkU2LW+eLRJ65eozeya6eO WpAQL0KrjvB/dpTgZ5ncYRoCH8qoDxUQoMcsnSlo74jqESdErf7/Pn9Jmi2t7xwXuCZG fGeLPCUjqGDQ4GX/gkdpsxFPrlyw2e/gH0/rJVHJGRzUWfF7FwqPVduYTFQsIgmx+p+L e8Gw== X-Gm-Message-State: APjAAAUVfBTaozBwrrGW+w59lSUXXzHmdGJ5Wy6GIwCq2WPnbASTWcVe 8Hu1Zy6ldB3EaCtrW/trY5tWU9TjCD65z9MVAcupSA== X-Google-Smtp-Source: APXvYqyVBpfyqKmXVyn2YNmR2BU6zjqhhsO8FplLuj62TJ6bRCC2KzHUqEAKuTa9w3KfcFIh3fHT6UdZPR6A3H6Zylc= X-Received: by 2002:a9d:69d6:: with SMTP id v22mr3609181oto.212.1562646538061; Mon, 08 Jul 2019 21:28:58 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Khushboo Vashi Date: Tue, 9 Jul 2019 09:58:48 +0530 Message-ID: Subject: Re: [GSoC] Finalized First Patch To: Dave Page Cc: Yosry Muhammad , pgadmin-hackers , Akshay Joshi Content-Type: multipart/alternative; boundary="000000000000cf9fd9058d37ffdf" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000cf9fd9058d37ffdf Content-Type: text/plain; charset="UTF-8" On Fri, Jul 5, 2019 at 4:31 PM Dave Page wrote: > Hi > > On Fri, Jul 5, 2019 at 6:28 AM Yosry Muhammad wrote: > >> - The patch won't apply with "git apply" and only partially applies with >>> patch -p0. Please "git add" all your changes and new files in your repo, >>> and then run "git diff --cached --binary", which should create a usable >>> patch. You can then un-stage your changes. >>> >>> >> That was due to new image. I made an applicable one with the below >> modifications, please find it attached. >> > > Thanks! > > >> >> >>> - execute_query_utils.py is somewhat lacking in file header and any >>> comments. >>> >>> >> Added. >> >> - There is commented-out code in sqleditor.js >>> >> >> This is the call that adds queries that are generated by pgAdmin to the >> query history. I commented it instead of removing it as I will add it later >> with some modifications when I add the checkbox. >> > > Sure, but we won't commit commented-out code. It just makes things messy > until such time as it gets used (or more often, does not). > > >> - On reflection, I don't think the "Data saved successfully, you still >>> need to commit changes to the database." is prominent enough - in testing, >>> I found it very easy to miss. That might also be compounded by the fact >>> that "Alert on uncommitted transactions?" doesn't seem to be working for >>> me. I get the "Save text" prompt to save the query text, but if I say no to >>> that, the Query Tool instance closes with no further warning, despite >>> having a transaction in progress. >>> >> >> I made a separate notification for the uncommitted save to make it more >> visible, check it out. >> > > That's definitely clearer now. It avoids the "blindness" caused by the > fact that you always get the green message. > > >> I tried the scenario you provided and the uncommitted alert worked fine, >> could you please try again and tell me the exact scenario where that >> happened? >> > > Hmm, it's working fine for me today too. Definitely wasn't yesterday > though! > > I'm going to make some minor tweaks to the wording of the docs before I > commit (as well as removing the commented-out code), but I think this is > good to go, once it's had another review. Khushboo, please take a look as > soon as you can. > > Sure. I am on it. > Thanks! > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > --000000000000cf9fd9058d37ffdf Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Fri, Jul 5, 2019 at 4:31 PM Dave P= age <dpage@pgadmin.org> wrot= e:
Hi

On Fri, Jul 5, 2019 at 6:28 AM Yosry Muhammad <yosrym93@gmail.com> wrote:=
=
- The patch won't apply with "git apply"= ; and only partially applies with patch -p0. Please "git add" all= your changes and new files in your repo, and then run "git diff --cac= hed --binary", which should create a usable patch. You can then un-sta= ge your changes.


That w= as due to new image. I made an applicable one with the below modifications,= please find it attached.

=
Thanks!
=C2=A0
=C2= =A0
- execute_query_utils.py is somewhat lacking in file header and any c= omments.


Added.

- There is commented-out code in sqleditor.js

This is the call that adds queries that are g= enerated by pgAdmin to the query history. I commented it instead of removin= g it as I will add it later with some modifications when I add the checkbox= .

Sure, but we won= 9;t commit commented-out code. It just makes things messy until such time a= s it gets used (or more often, does not).
=C2=A0
=
- On reflection, I don't think the "Data saved successfully, = you still need to commit changes to the database." is prominent enough= - in testing, I found it very easy to miss. That might also be compounded = by the fact that "Alert on uncommitted transactions?" doesn't= seem to be working for me. I get the "Save text" prompt to save = the query text, but if I say no to that, the Query Tool instance closes wit= h no further warning, despite having a transaction in progress.

I made a separate notification for the= uncommitted save to make it more visible, check it out.
<= /blockquote>

That's definitely clearer now. It avoid= s the "blindness" caused by the fact that you always get the gree= n message.
=C2=A0
I tried the scenar= io you provided and the uncommitted alert worked fine, could you please try= again and tell me the exact scenario where that happened?

Hmm, it's working fine for me today t= oo. Definitely wasn't yesterday though!

I'= m going to make some minor tweaks to the wording of the docs before I commi= t (as well as removing the commented-out code), but I think this is good to= go, once it's had another review. Khushboo, please take a look as soon= as you can.

Sure. I am o= n it.=C2=A0
Thanks!=C2=A0

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake<= br>
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company=
--000000000000cf9fd9058d37ffdf--