Re: [pgAdmin4] [Patch]: Language Module

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4] [Patch]: Language Module
Date: 2016-02-19 10:48:17
Message-ID: CANxoLDecXboxgJjdTsDCq1SKahacoAFCcANrHAjU2dcZfVGFJw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi All

On Mon, Feb 15, 2016 at 3:47 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

>
>
> On Mon, Feb 15, 2016 at 7:29 AM, Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi All
>>
>> Attached is the new patch after fixing review comments given by Dave.
>>
>> On Fri, Feb 12, 2016 at 10:04 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On Fri, Feb 12, 2016 at 11:21 AM, Harshal Dhumal <
>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi,
>>>>
>>>> New language patch is working fine.
>>>> Ashesh - Can you please review it ?
>>>> If it looks good then can you please commit ?
>>>>
>>>> Thank you,
>>>> Harshal
>>>>
>>>
>>> A few comments:
>>>
>>> - We omitted CREATE LANGUAGE, as most are now extensions. Shouldn't we
>>> also therefore omit DROP LANGUAGE?
>>>
>> Fixed.
>>
>>> - Please leave a blank line between the file header and python code in
>>> python files.
>>>
>> Fixed.
>>
>>> - The pydoc module description is missing from __init__.py
>>>
>> Fixed.
>>
>>> - What about the Dependents and Dependencies tabs?
>>>
>> Infrastructure is not ready, so not implemented yet.
>>
>
> The first person that needs it should be working on it! I don't want to be
> committing partially completed features, otherwise we'll have to come back
> and revisit them all when the infrastructure is done.
>
> Having done it before for pgAdmin 3, I don't imagine it's hard. It's
> really just the same as the Statistics view. Please take a look at it
> (assuming noone else is already).
>

Fixed. Attached is the modified patch with support of Dependents and
Dependencies tabs. Please review it.

Thanks.
>
>
>>
>>> - There are few, if any comments in the Javascript. There should be at
>>> least minimal comments there to note the purpose of each logical code block.
>>>
>> Fixed.
>>
>> --
>> *Akshay Joshi*
>> *Principal Software Engineer *
>>
>>
>>
>> *Phone: +91 20-3058-9517 <%2B91%2020-3058-9517>Mobile: +91 976-788-8246*
>>
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
*Akshay Joshi*
*Principal Software Engineer *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*

Attachment Content-Type Size
Languages_v3.patch application/octet-stream 39.2 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Sanket Mehta 2016-02-19 11:03:50 Re: patch for cast module
Previous Message Akshay Joshi 2016-02-19 10:17:02 [pgAdmin4][Patch]: Dependecies/Dependents functionality