Re: [pgadmin4] Edb package support.

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.vashi@enterprisedb.com> wrote:

On Wed, Aug 17, 2016 at 4:35 PM, Dave Page <dpage@pgadmin.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.dhumal@enterprisedb.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@postgresql.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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers