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
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 |