MIME-Version: 1.0
References: 
 <CAFOhELd8YNNq5Ot-qUD0QOP_Of=iKadaTrC9trJVLENttErGsg@mail.gmail.com>
In-Reply-To: 
 <CAFOhELd8YNNq5Ot-qUD0QOP_Of=iKadaTrC9trJVLENttErGsg@mail.gmail.com>
From: Dave Page <dpage@pgadmin.org>
Date: Thu, 21 Jan 2021 09:47:32 +0000
Message-ID: 
 <CA+OCxozOnj+RFycxhmmni-mYXAn-Jw58bwWgXnFMqDkrXimSnQ@mail.gmail.com>
Subject: Re: [pgAmin4][Patch] - Kerberos support for Windows Installer
To: Khushboo Vashi <khushboo.vashi@enterprisedb.com>
Cc: pgadmin-hackers <pgadmin-hackers@postgresql.org>
Content-Type: multipart/alternative; boundary="00000000000093bc2805b965f698"
Precedence: bulk

--00000000000093bc2805b965f698
Content-Type: text/plain; charset="UTF-8"

Hi

On Thu, Jan 21, 2021 at 7:11 AM Khushboo Vashi <
khushboo.vashi@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached patch to add the Kerberos support in the Windows
> installer script.
>

A couple of comments:

- The file names should be in lower case in the COPY lines. It will still
work of course, but for consistency and style...
- There's no check that the environment variable is set to a valid value.
For others, we have code block like:

IF NOT EXIST "%PGADMIN_QT_DIR%" (
    ECHO !PGADMIN_QT_DIR! does not exist.
    ECHO Please install Qt and set the PGADMIN_QT_DIR environment variable.
    EXIT /B 1
)


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

EDB: http://www.enterprisedb.com

--00000000000093bc2805b965f698
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi</div><br><div class=3D"gmail_quote"><d=
iv dir=3D"ltr" class=3D"gmail_attr">On Thu, Jan 21, 2021 at 7:11 AM Khushbo=
o Vashi &lt;<a href=3D"mailto:khushboo.vashi@enterprisedb.com">khushboo.vas=
hi@enterprisedb.com</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quot=
e" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204)=
;padding-left:1ex"><div dir=3D"ltr">Hi,<div><br></div><div>Please find the =
attached patch to add the Kerberos support in the Windows installer script.=
</div></div></blockquote><div><br></div><div>A couple of comments:</div><di=
v><br></div><div>- The file names should be in lower case in the COPY lines=
. It will still work of course, but for consistency and style...</div><div>=
- There&#39;s no check that the environment variable is set to a valid valu=
e. For others, we have code block like:</div><div><br></div><div><pre style=
=3D"background-color:rgb(43,43,43);color:rgb(169,183,198);font-family:&quot=
;JetBrains Mono&quot;,monospace;font-size:12pt"><span style=3D"color:rgb(20=
4,120,50)">IF NOT EXIST </span><span style=3D"color:rgb(106,135,89)">&quot;=
%PGADMIN_QT_DIR%&quot; </span>(<br>    <span style=3D"color:rgb(204,120,50)=
">ECHO </span><span style=3D"color:rgb(106,135,89)">!PGADMIN_QT_DIR! does n=
ot exist.<br></span><span style=3D"color:rgb(106,135,89)">    </span><span =
style=3D"color:rgb(204,120,50)">ECHO </span><span style=3D"color:rgb(106,13=
5,89)">Please install Qt and set the PGADMIN_QT_DIR environment variable.<b=
r></span><span style=3D"color:rgb(106,135,89)">    </span>EXIT /B 1<br>)</p=
re></div><div>=C2=A0</div></div>-- <br><div dir=3D"ltr" class=3D"gmail_sign=
ature"><div dir=3D"ltr">Dave Page<br>Blog: <a href=3D"http://pgsnake.blogsp=
ot.com" target=3D"_blank">http://pgsnake.blogspot.com</a><br>Twitter: @pgsn=
ake<br><br>EDB: <a href=3D"http://www.enterprisedb.com" target=3D"_blank">h=
ttp://www.enterprisedb.com</a><br><br></div></div></div>

--00000000000093bc2805b965f698--