MIME-Version: 1.0
In-Reply-To: 
 <CAM5-9D9LD+e0U2O59qdHc8zSKmazT0OytybFOQnNKLsr5SxkrA@mail.gmail.com>
References: 
 <CAM5-9D9k1MGYxhf-pobDbeDJxOttVawje=JzFesEMgD=fxRR+w@mail.gmail.com>
	<CAM5-9D9P6tfPajz1mUi1B03+ypLKsebehkaxWfV_C6k=ZjCSPg@mail.gmail.com>
	<CACCA4P3JuzVzoq7DFmyFzjbKaE3uf-P1+4_zkBO6RfxTB-VVSQ@mail.gmail.com>
	<CAM5-9D_jZUQMinr4s3upX3ktSM6AV_NWQ-RMQMjyroWMNL3bcg@mail.gmail.com>
	<CAM5-9D81Yg5ofY0bjgZvSDM7Gt1t7bTuqgMSmExx1086tMT7Qg@mail.gmail.com>
	<CAM5-9D-c2s9bfxVgoZMHbWNsvsiQzyHSMtx2revHz-2vaDYTJw@mail.gmail.com>
	<CACCA4P2OFF8mHsRocYpq+YFV5mMZTw1yLOW-FYsHuOvrm7G0UQ@mail.gmail.com>
	<CAM5-9D-FMDcV8L1FbPiEU4pCeOO1dcLu-62r5CHRuO_ks9hiSw@mail.gmail.com>
	<CACCA4P27KHmsiWAJzOy3rL+9pLyg1jr=c27kbK97g+N3nAZZyQ@mail.gmail.com>
	<CAM5-9D9LD+e0U2O59qdHc8zSKmazT0OytybFOQnNKLsr5SxkrA@mail.gmail.com>
Date: Mon, 15 Feb 2016 11:07:26 +0000
Message-ID: 
 <CA+OCxowvwaqPNzn317d4CEa3HC2j8RfFNPH2SNNuJ+QpaAt=0A@mail.gmail.com>
Subject: Re: [pgAdmin4] [Patch]: Extension Module
From: Dave Page <dpage@pgadmin.org>
To: Surinder Kumar <surinder.kumar@enterprisedb.com>
Cc: Neel Patel <neel.patel@enterprisedb.com>,
	pgadmin-hackers <pgadmin-hackers@postgresql.org>
Content-Type: multipart/alternative; boundary=001a11339b949ffeda052bcd0531
Precedence: bulk
Sender: pgadmin-hackers-owner@postgresql.org

--001a11339b949ffeda052bcd0531
Content-Type: text/plain; charset=UTF-8

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.
- s/}else{/} else {/
- Dependency/depends display is missing (see previous email to Akshay).
This is essential for this node!
- There's no pydoc comment introducing __init__.py
- Shouldn't "data='-- Modified SQL --'," be "data=gettext('-- Modified SQL
--'),"?


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

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

--001a11339b949ffeda052bcd0531
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Hi<div class=3D"gmail_extra"><br><div class=3D"gmail_quote=
">On Mon, Feb 15, 2016 at 9:55 AM, Surinder Kumar <span dir=3D"ltr">&lt;<a =
href=3D"mailto:surinder.kumar@enterprisedb.com" target=3D"_blank">surinder.=
kumar@enterprisedb.com</a>&gt;</span> wrote:<br><blockquote class=3D"gmail_=
quote" style=3D"margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-=
color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir=
=3D"ltr">Hi,=C2=A0<div><br></div><div>PFA patch with following changes:</di=
v><div><ol><li>Added &quot;Create Extension&quot; menu item in context menu=
 of Database node.</li><li>Added a new method &quot;node_node&quot; in Exte=
nsionModule class. If a node has child, returns True, otherwise False.</li>=
<li>Fixed an issue in which icon won&#39;t display in create extension link=
 in context menu.</li><li>Added Docstring for the class and methods in pyth=
on file and proper commenting in js file.</li><li>Followed PEP-08 coding co=
nventions.</li></ol></div></div></blockquote><div><br></div><div>I haven=
9;t tested this, but a few initial comments:</div><div><br></div><div>- The=
 commenting of the JS code is better than I&#39;ve seen in other patches \o=
/, but the commenting style is inconsistent. We should use /* */ for multi-=
line comments, and // for single line.</div><div>- =C2=A0The JS code could =
use some carefully introduced blank lines to help make it more readable.</d=
iv><div>- s/}else{/} else {/</div><div>- Dependency/depends display is miss=
ing (see previous email to Akshay). This is essential for this node!</div><=
div>- There&#39;s no pydoc comment introducing __init__.py</div><div>- Shou=
ldn&#39;t &quot;data=3D&#39;-- Modified SQL --&#39;,&quot; be &quot;data=3D=
gettext(&#39;-- Modified SQL --&#39;),&quot;?</div><div><br></div></div><di=
v><br></div>-- <br><div class=3D"gmail_signature">Dave Page<br>Blog: <a hre=
f=3D"http://pgsnake.blogspot.com" target=3D"_blank">http://pgsnake.blogspot=
.com</a><br>Twitter: @pgsnake<br><br>EnterpriseDB UK: <a href=3D"http://www=
.enterprisedb.com" target=3D"_blank">http://www.enterprisedb.com</a><br>The=
 Enterprise PostgreSQL Company<br></div>
</div></div>

--001a11339b949ffeda052bcd0531--