Re: [pgAdmin4] [Patch]: Extension Module - Mailing list pgadmin-hackers

From Dave Page
Subject Re: [pgAdmin4] [Patch]: Extension Module
Date
Msg-id CA+OCxoyeCxRWgBa=Yq_h6uAkgv9oXWfRkK-r9JiAaFSVnSOmpg@mail.gmail.com
Whole thread Raw
In response to Re: [pgAdmin4] [Patch]: Extension Module  (Surinder Kumar <surinder.kumar@enterprisedb.com>)
List pgadmin-hackers
Thanks - committed.

On Tue, Feb 23, 2016 at 11:07 AM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:
Hi,

PFA patch with changes suggested by Dave

Please review the patch and let me know for any comments.

On Mon, Feb 15, 2016 at 4:37 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Mon, Feb 15, 2016 at 9:55 AM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:
Hi, 

PFA patch with following changes:
  1. Added "Create Extension" menu item in context menu of Database node.
  2. Added a new method "node_node" in ExtensionModule class. If a node has child, returns True, otherwise False.
  3. Fixed an issue in which icon won't display in create extension link in context menu.
  4. Added Docstring for the class and methods in python file and proper commenting in js file.
  5. Followed PEP-08 coding conventions.

I haven't tested this, but a few initial comments:

- The commenting of the JS code is better than I've seen in other patches \o/, but the commenting style is inconsistent. We should use /* */ for multi-line comments, and // for single line.
-  The JS code could use some carefully introduced blank lines to help make it more readable.
Done 
- s/}else{/} else {/
Done 
- Dependency/depends display is missing (see previous email to Akshay). This is essential for this node!
Implemented dependency and depends 
- There's no pydoc comment introducing __init__.py
Added pydoc
- Shouldn't "data='-- Modified SQL --'," be "data=gettext('-- Modified SQL --'),"?
Yes, it should be. Fixed. 


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company




--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

pgadmin-hackers by date:

Previous
From: Dave Page
Date:
Subject: pgAdmin 4 commit: Add support for extensions.
Next
From: Dave Page
Date:
Subject: Re: patch for cast module