Re: [pgAdmin][RM3794]:Allow User to Change Database Connection from an Open Query Tool Tab

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Nikhil Mohite <nikhil(dot)mohite(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin][RM3794]:Allow User to Change Database Connection from an Open Query Tool Tab
Date: 2020-09-29 06:01:37
Message-ID: CANxoLDdkvhUE3pohPPgyyz2tYwYej+VAkxUE-e09JBxd3Aeh0w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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(dot)mohite(at)enterprisedb(dot)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(dot)joshi(at)enterprisedb(dot)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(dot)mohite(at)enterprisedb(dot)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(dot)joshi(at)enterprisedb(dot)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(dot)mohite(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Team,
>>>>>
>>>>> Regarding RM-3794 <https://redmine.postgresql.org/issues/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* <https://www.enterprisedb.com/>
>>>>> *Mob.No: +91-7798364578.*
>>>>>
>>>>
>>>>
>>>> --
>>>> *Thanks & Regards*
>>>> *Akshay Joshi*
>>>> *pgAdmin Hacker | Sr. Software Architect*
>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>
>>>> *Mobile: +91 976-788-8246*
>>>>
>>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Sr. Software Architect*
>> *EDB Postgres <http://edbpostgres.com>*
>>
>> *Mobile: +91 976-788-8246*
>>
>

--
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Sr. Software Architect*
*EDB Postgres <http://edbpostgres.com>*

*Mobile: +91 976-788-8246*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2020-09-29 09:39:24 pgAdmin 4 commit: Fixed code smells 'Define a constant instead of dupli
Previous Message Akshay Joshi 2020-09-29 05:58:35 pgAdmin 4 commit: Ensure that 'ctrl + a' shortcut does not move the cur