Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kN8i5-00032U-4u for pgadmin-hackers@arkaria.postgresql.org; Tue, 29 Sep 2020 06:01:57 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1kN8i3-0002Zz-JT for pgadmin-hackers@arkaria.postgresql.org; Tue, 29 Sep 2020 06:01:55 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kN8i3-0002Zs-Cj for pgadmin-hackers@lists.postgresql.org; Tue, 29 Sep 2020 06:01:55 +0000 Received: from mail-io1-xd2d.google.com ([2607:f8b0:4864:20::d2d]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1kN8hz-0006t2-15 for pgadmin-hackers@postgresql.org; Tue, 29 Sep 2020 06:01:54 +0000 Received: by mail-io1-xd2d.google.com with SMTP id u6so3592694iow.9 for ; Mon, 28 Sep 2020 23:01:50 -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=8f7OgALpos5AXOXxNUu5oq0HkvI1YRBulioWYfAfnCE=; b=RpFqozmUEcerLJiZDXo9ItfWylCmN+ONcdnZEgUlk22bn8xnF+FO5ROAe7cin4Dc3x wqqnbHqFNLu5jtHR9DzF9m9VEXQAm+l349roVa96YybIS3G0B3lVoazdIvDhcyS7xpo1 8VyGih4ntN21Q5+MDmQKxz8LGQ1jGnH8i0XCgOdNiY6EWN2cnuxM7tqgJRnZnWy0gv8S Zn9Lm0mX+ZgbX00wdR6TdJOuCfANvZl0OxnGlMi9EliaRuJ253RwDbNfeBw7Yf2b4sHz yYff57XijEVm++QK7Nkj+ZqeB6lUIQECP2qOqbtyGVmDCJYvmAFNbMo/K3WnQZb9wW/Z dSdw== 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=8f7OgALpos5AXOXxNUu5oq0HkvI1YRBulioWYfAfnCE=; b=HZ8LJELZESvyKidEEpGMl4yxQchIqxsXmAtva+lKc1O/60QZQIRtoSDurzGTAl/Dr4 NT0oTsmQ1QkxDhKdWaAv1UirY3kq9X7zG6dhsMJBPnhvy8z8m5yAJwLQI5p8le+3eaG1 DARtg4/K5963/ZX2R6k2itlJlE3GTD6p1z4J5CLA+5Ns4bjarVqY/cA4EcbUZd0M+IXM cvnPGUbolY1/AIqgXvXLL9pRGUGsRNZeRvux/L/ijZ7KHSB+aysDsqc9xeCmzi+8jiZ6 FHVBS/bltIFlaZwFT2Ab5ADs9Hj9h/QWHPqYMXZODhlbOz9Rwe1GYfRPBQPJg08+Zi4A 5QFw== X-Gm-Message-State: AOAM532v0hmlVL1VG956SrLkVgfkeVskKC3NRt0mn9MTxS9GurTpb9N9 Mh3G4oMx8alQcj3M41C+ovuWLBMBfSpEwj31nlc2w2jZ06u+2SsJoS6lsxjYUe40c+IFkph6ur/ PvFZBa5w/Bxpp4X4Lb/P66kdFfA8zaNFwuEtzIvZURwthX0/SY6lfF8SHVT/1WCCoOYfEnoqcDb +smHgfHRShXEGG9QBltuX0F0/W9B1XD/8nSab76YzfZajL80owbGpdQh1Ant+Ympo= X-Google-Smtp-Source: ABdhPJxGUmqWQGPGYEhqd/sMZoRNjWNVaUlnl0KaslkIFsf8BuSP19AlZ5M8vrxyWMo5Mj7SlKasRheL7pIATg/drIg= X-Received: by 2002:a05:6638:d46:: with SMTP id d6mr1802440jak.20.1601359308515; Mon, 28 Sep 2020 23:01:48 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Akshay Joshi Date: Tue, 29 Sep 2020 11:31:37 +0530 Message-ID: Subject: Re: [pgAdmin][RM3794]:Allow User to Change Database Connection from an Open Query Tool Tab To: Nikhil Mohite Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000be02e905b06d84f1" X-CLOUD-SEC-AV-Info: enterprisedb,google_mail,monitor X-CLOUD-SEC-AV-Sent: true X-Gm-Spam: 0 X-Gm-Phishy: 0 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000be02e905b06d84f1 Content-Type: text/plain; charset="UTF-8" Hi Nikhil Your patch introduces 1 new Bug and 13 new code smells, please fix those and resend the patch. On Mon, Sep 28, 2020 at 7:31 PM Nikhil Mohite < nikhil.mohite@enterprisedb.com> wrote: > Hi Akshay, > > I have resolved code conflict issues and sonarqube issues. > PFA updated patch. > > Regards, > Nikhil Mohite. > > On Mon, Sep 28, 2020 at 5:58 PM Akshay Joshi < > akshay.joshi@enterprisedb.com> wrote: > >> Hi Nikhil >> >> The patch is not applying, rebase, and send it again. Please check your >> code should not create any new SonarQube issues. >> >> On Mon, Sep 28, 2020 at 11:20 AM Nikhil Mohite < >> nikhil.mohite@enterprisedb.com> wrote: >> >>> Hi Akshay, >>> >>> I have resolved all the review comments and also updated the test cases >>> as per the new implementation. >>> >>> PFA updated patch. >>> >>> >>> >>> On Mon, Sep 21, 2020 at 5:24 PM Akshay Joshi < >>> akshay.joshi@enterprisedb.com> wrote: >>> >>>> Hi Nikhil >>>> >>>> Following are the initial review comments: >>>> >>>> - Open View/Edit data on any table and click on the same database >>>> connection and then click on the Execute button. Got "get_primary_keys() >>>> takes 1 positional argument but 2 were given" error. >>>> - In my opinion, we should hide the option to change the database >>>> connection for View/Edit Data. >>>> - If the user clicks on the same database connection multiple times >>>> then no need to change the backend connection and transaction id. Add >>>> validation at the backend, no action required in this case. >>>> - The role option is missing from the "connect to server" dialog. >>>> - The Password field should not be there on the "connect to server" >>>> dialog. Sometimes we saved the password so asking a password every time is >>>> not correct. Check the pgAdmin 3 behavior. >>>> >>>> Code review still remains. >>>> >>>> On Thu, Sep 17, 2020 at 4:15 PM Nikhil Mohite < >>>> nikhil.mohite@enterprisedb.com> wrote: >>>> >>>>> Hi Team, >>>>> >>>>> Regarding RM-3794 allow >>>>> the user to change the database connection from an open query tool: >>>>> I have implemented the feature and also added documentation for it. >>>>> >>>>> PFA patch. >>>>> >>>>> -- >>>>> *Thanks & Regards,* >>>>> *Nikhil Mohite* >>>>> *Software Engineer.* >>>>> *EDB Postgres* >>>>> *Mob.No: +91-7798364578.* >>>>> >>>> >>>> >>>> -- >>>> *Thanks & Regards* >>>> *Akshay Joshi* >>>> *pgAdmin Hacker | Sr. Software Architect* >>>> *EDB Postgres * >>>> >>>> *Mobile: +91 976-788-8246* >>>> >>> >> >> -- >> *Thanks & Regards* >> *Akshay Joshi* >> *pgAdmin Hacker | Sr. Software Architect* >> *EDB Postgres * >> >> *Mobile: +91 976-788-8246* >> > -- *Thanks & Regards* *Akshay Joshi* *pgAdmin Hacker | Sr. Software Architect* *EDB Postgres * *Mobile: +91 976-788-8246* --000000000000be02e905b06d84f1 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi=C2=A0Nikhil

Your patch introduces 1 = new Bug and 13 new code smells, please fix those and resend the patch.


Hi=C2=A0Nik= hil

The patch is not applying, rebase, and send it again= . Please check your code should not create any new SonarQube issues.
<= /div>

Hi=C2=A0Nikhil

Following are the initi= al review comments:
  • Open View/Edit data on any table and click = on the same database connection and then click on the Execute button. Got &= quot;get_primary_keys() takes 1 positional argument but 2 were given" = error.
  • In my opinion, we should hide the option to change the datab= ase connection for View/Edit Data.
  • If the user clicks on the same d= atabase connection multiple times then no need to change the backend connec= tion and transaction id. Add validation at the backend, no action required = in this case.
  • The role option is missing from the "connect to = server" dialog.
  • The Password field should not be there on the = "connect to server" dialog. Sometimes we saved the password so as= king a password every time is not correct. Check the pgAdmin 3 behavior.
Code review still remains.

On Thu, Sep 17, 2020 at 4:15 P= M Nikhil Mohite <nikhil.mohite@enterprisedb.com> wrote:
Hi Team,
Regarding RM-3794 allow the=C2=A0user to change the database= connection from an open query tool:
I have implemented the featu= re and also added documentation for it.

PFA patch.=

--
Thanks & Regards,
Nikhil Mohite
Software Engineer.
Mob.No: +91-7798364578.
=


--
Thank= s & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Ar= chitect
EDB Postgres=
Mobile: +91 976-788-8246



--
Thank= s & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Ar= chitect
EDB Postgres=
Mobile: +91 976-788-8246



--
Thank= s & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Ar= chitect
EDB Postgres=
Mobile: +91 976-788-8246

--000000000000be02e905b06d84f1--