mark.cave-ayland at siriusit.co.uk
Tue Dec 16 15:55:52 PST 2008
Paul Ramsey wrote:
> FYI, I've committed in the early bits of code on my line crossing
> project. Most interesting to the development community is the contents
> of 'cunit', which is a cunit test harness directly working on
> liblwgeom. It was really invaluable for testing the code, I must say.
> Good work Mark on breaking out liblwgeom, it has all worked as
> advertised so far.
Cool. Glad to see things are working out for your project, and you like
the new code :) The commit looks good, although I'd like to point out a
few comments - these are probably due to the lack of liblwgeom
documentation on my part, but there we are...
At the moment I see you are using the in-built functions. As this is
likely to be quite common, I created a function called
lwgeom_install_default_allocators() which uses these "standard" defaults
- see shp2pgsql/pgsql2shp for a quick example.
- naming convention
I see you've gone for camelCase naming in lwalgorithm, whereas most of
the remainder of liblwgeom goes for underscore names with a lw prefix.
Although this probably amounts to a separate discussion, I would suggest
that trying to match the existing convention would be a better starting
point until then. (I personally am not too bothered either way, so I
guess it's a case of making a decision and sticking to it).
- debug levels
The initial scheme I used for debug levels within liblwgeom/PostGIS was
documented here: http://svn.refractions.net/postgis/trunk/lwgeom/DEBUG
but I see you've picked slightly different numbers. Again, since this is
all new then if you decide we need to shuffle this around a bit, then we
can accommodate this.
- header/C files
Some of them don't have any copyright notices. It would be nice to break
out the function prototypes into separate files declared extern (as you
have done with lwalgorithm.h) so we can more carefully control what is
internal and what is external API. Unfortunately I haven't quite found
the time to do this, and at the moment the reward is fairly low...
> I'm interested in thoughts on how we should handle cunit tests, or if
> we should even have them at all. Right now there is a hardcode
> dependency on have cunit installed in /usr/local, but obviously it is
> still early days.
Yeah, that's fine. If everyone is happy then we can add the
executable/header detection code into the autoconf build system so that
it "just works"TM.
Sirius Corporation - The Open Source Experts
T: +44 870 608 0063
More information about the postgis-devel