Re: pgAgent: C++ Port - Patch Review

From: Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Linreg <linreg(at)gmx(dot)net>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: pgAgent: C++ Port - Patch Review
Date: 2013-10-24 05:41:21
Message-ID: CAMcbDBEsn2zK+Xup+DS8yU2js1XdkY6AZDrrQnLCw7V_jzozBA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

Below are the review comments. Compile and tested in Linux.

1) During compilation we got the following error in "connection.h"

error: pgsql/libpq-fe.h: No such file or directory

To solve the above error we changed "#include <libpg-fe.h>" instead of
"#include <pgsql/libpq-fe.h>" and it is working fine. Correct me if it is
some configuration issue.

2) We are getting the below warning in unix.cpp file which needs to fix.

warning: the use of `tmpnam' is dangerous, better use `mkstemp'.

3) Some of the code is commented and not used so remove the unnecessary
code.

4) README file should be modified according to new changes.

5) In Execute() method in Job.cpp file, we should have to delete "steps"
from wherever we are returning otherwise it will create the memory leak.

Thanks,
Neel Patel

On Fri, Oct 18, 2013 at 5:35 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Neel, can you review this patch please?
>
> Thanks.
>
> On Mon, Oct 14, 2013 at 9:24 AM, Linreg <linreg(at)gmx(dot)net> wrote:
> > Hello Dave,
> >
> >
> >
> > Excuse my long absence. I had a lot to do.
> >
> > Here is my patch.
> >
> > Please note the following:
> >
> > - The Windows part does not work. I'll make it if you accept this
> changes.
> >
> > - The start parameters have a different output format
> >
> > (all old parameter are the same. NO CHANGES from user is needed! But you
> >
> > have long-options and no atoi/atol function any more)
> >
> > - In the CMAKE file I have WX replaced by BOOST.
> >
> > - I have re-impemented Connection-Pooling
> >
> > - I have removed CSV-Logoutput-Format
> >
> > - This Changes i send later.
> >
> >
> >
> > Thomas Steffen
> >
> >
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Linreg 2013-10-24 09:29:18 Re: pgAgent: C++ Port - Patch Review
Previous Message Neel Patel 2013-10-24 05:13:11 Re: [pgadmin-support] pgAdmin 1.18.0 + slony-I 2.2.0 + PG 9.3