From: | Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com> |
---|---|
To: | Dave Page <dpage(at)pgadmin(dot)org> |
Cc: | Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: [pgadmin4] Edb package support. |
Date: | 2016-08-18 07:04:35 |
Message-ID: | CAFOhELf+9fdC3TZOkGvM2EcNkkyH7j2CuxOV5GPOP85bEtXCNQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
Hi Harshal,
Please see below review comments.
1. Please make one line space between every SQL block as per pgAdmin 4
standard.
2. The RE SQL of package is not correct. The package name is displayed
twice as shown below
CREATE OR REPLACE PACKAGE enterprisedb.empinfo
IS
edb.empinfo is
emp_name character varying(10);
procedure get_name(p_empno numeric);
function display_counter() return integer;
END empinfo;
Same applies for CREATE OR REPLACE PACKAGE BODY also.
3. The RE SQL of functions, procedures and variable are not correct.
Currently it is showing the entire package body.
4. The Create scripts are incorrect for the package and its sub-nodes. If I
execute that script, it gives me an error.
5. Not able to update the package header as well as body as in the edit
window the script values are incorrect. So, at the end it will create the
wrong SQL.
6. Please check and change the comments for all the JS files if they are
copied from another node. Ex: package.js is showing sequence node comments.
7. Please maintain 4 tab indentation for SQL templates
NOTE: I have not repeated Dave's comments, so please incorporate those
comments in this list.
Thanks,
Khushboo
On Thu, Aug 18, 2016 at 10:50 AM, Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>
> On Wed, Aug 17, 2016 at 4:35 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi,
>>
>> I did some quick testing, and found the following issues:
>>
>> - Package ACL is not shown in properties
>>
>> - Package header and body are not shown in properties
>>
>> - System package? is not shown in properties
>>
>> - The header for the RE SQL on a package procedure should read:
>>
>> -- Package Procedure
>>
>> (not -- Package Function)
>>
>> Can you also have someone (Khushboo?) do a code review, if that hasn't
>> happened already?
>>
>>
> Sure, will do this.
>
>
>> Thanks.
>>
>> On Tue, Aug 16, 2016 at 1:31 PM, Harshal Dhumal
>> <harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>> > Hi,
>> >
>> > PFA initial patch for edb packages.
>> >
>> > Other changes:
>> > 1] Added 'canEdit' flag for node to enable/disabled node edit. (by
>> default
>> > it's enable)
>> > 2] Privileges: Do not show 'ALL' in query if object has only one
>> applicable
>> > privilege instead show that privilege.
>> >
>> >
>> > --
>> > Harshal Dhumal
>> > Software Engineer
>> >
>> > EnterpriseDB India: http://www.enterprisedb.com
>> > The Enterprise PostgreSQL Company
>> >
>> >
>> > --
>> > Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org)
>> > To make changes to your subscription:
>> > http://www.postgresql.org/mailpref/pgadmin-hackers
>> >
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>> --
>> Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>
>
>
From | Date | Subject | |
---|---|---|---|
Next Message | Priyanka Shendge | 2016-08-18 08:54:41 | Re: pgAdmin IV : Unittest modular patch(database child nodes) and trigger function |
Previous Message | Murtuza Zabuawala | 2016-08-18 06:24:04 | PATCH: To fix the issue in configuration error message (pgAdmin4) |