Re: GSoC - Patch (Schema Difference Sync+ Visualization)

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Guillaume Lelarge <guillaume(at)lelarge(dot)info>
Cc: adeel khan <ak1733(at)gmail(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: GSoC - Patch (Schema Difference Sync+ Visualization)
Date: 2010-08-17 14:27:50
Message-ID: AANLkTi=RSWV6+gvO0R8D03+poNkyD7xt5GmYumQx848Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Adeel,

On Mon, Aug 16, 2010 at 10:13 PM, Guillaume Lelarge
<guillaume(at)lelarge(dot)info> wrote:
> No, it isn't "too much" strict. It's just more strict, more precise than
> VC++.

Yeah, just a bit :-). The main issue though, is that VC++ is setup to
use precompiled headers which GCC isn't (I never got it to work
properly). This means that GCC will need #include statements to
include headers wherever needed, but VC++ doesn't.

Anyway, I started trying to build on Mac, but then ran out of time
unfortunately. I got as far as can be seen in the attached patch (this
is against the head of the GIT master branch).

A few further notes, most of which I've mentioned before and I know
were due to be fixed during final cleanup of the patch:

- The brace formatting is still not per project standards in some places.
- There are some filename casing issues as Guillaume noticed - eg.
pgadmin3.h vs. pgAdmin3.h (partly fixed in the patch)
- Missing const decoration in the XPM files (fixed in the patch)
- You cannot assume that the default schema is "public" when
generating SQL scripts. See pgDatabase::GetDefaultSchema()
- There are missing spaces all over the place. Use them to separate
things like #include and a filename as Guillaume spotted, and also
between braces, brackets and operators etc. for readability, eg:

void iSetAllowConnections(bool newVal){allowConnections=newVal;}

should be

void iSetAllowConnections(bool newVal) { allowConnections = newVal; }

- If there is code in a header that uses an external class, you need
to include the appropriate header - you can't just declare the class
name.
- At least the following files are missing from your latest patch:
svAddTable.cpp, svAddSchema.cpp and svConstraintDiff.cpp.

As Guillaume has suggested, now that the GSoC timeline pressures are
off, it seems like it would be a good idea to setup a repo on github
and host the project there until it's ready to commit. I don't think
it's *that* far off - most of the issues getting a basic build on
Linux/Mac could be resolved in a couple of hours - and most of the
rest is cosmetic.

Anyway, congratulations on reaching the end of the GSoC phase of your
project - I do hope you decide to keep at it in your spare time so we
can polish off the patch and get it into 1.12. For those that are
wondering, I have built and run previous patches on Windows, and the
tool looks pretty nice.

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

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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2010-08-17 14:28:18 Re: GSoC - Patch (Schema Difference Sync+ Visualization)
Previous Message Magnus Hagander 2010-08-17 14:08:22 Re: About our GSoC projects