[postgis-devel] State of shp2pgsql-gui in trunk

[Mark Leslie]

Mark,
I'm going to try and step up my game to bring this work up to speed
and continue to maintain it, but as I'm sure has been noted, I'm doing
a poor job of this lately.  If I prove insufficient, let me know and
I'll roll it back and stuff it into a spike or similar.

--
Mark

On 19 September 2010 22:55, Mark Cave-Ayland
<mark.cave-ayland at siriusit.co.uk> wrote:
> Hi folks,
>
> Having just been applying fixes to shp2pgsql-gui in both trunk and 1.5, I am
> getting quite concerned about the state of shp2pgsql-gui in trunk. Here are
> a few of the issues I have noticed with current trunk:
>
>
> - Multiple warnings at build-time
>
> gcc -g -O2  -fPIC -DPIC  -Wall -Wmissing-prototypes
> -I/home/pg83/rel-8.3.7/include -D_REENTRANT -I/usr/include/gtk-2.0
> -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo
> -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include
> -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/directfb
> -I/usr/include/libpng12    -o shp2pgsql-gui.o -c shp2pgsql-gui.c
> shp2pgsql-gui.c:155: warning: no previous prototype for ‘pgui_log_va’
> shp2pgsql-gui.c: In function ‘compare_columns’:
> shp2pgsql-gui.c:1000: warning: enumeration value ‘FTInvalid’ not handled in
> switch
> shp2pgsql-gui.c: In function ‘pgui_create_file_table’:
> shp2pgsql-gui.c:2369: warning: unused variable ‘bin_window’
> shp2pgsql-gui.c: At top level:
> shp2pgsql-gui.c:1675: warning: ‘pgui_action_null’ defined but not used
> shp2pgsql-gui.c: In function ‘pgui_action_shape_file_set’:
> shp2pgsql-gui.c:1713: warning: ‘status’ may be used uninitialized in this
> function
> shp2pgsql-gui.c: In function ‘pgui_action_handle_tree_edit’:
> shp2pgsql-gui.c:666: warning: ‘status’ may be used uninitialized in this
> function
>
>
> - Multiple GTK assertions triggered at run time
>
> e.g.
>
> (shp2pgsql-gui:29945): GLib-GObject-WARNING **:
> /build/buildd-glib2.0_2.22.4-1~bpo50+1-amd64-SJ3EDw/glib2.0-2.22.4/gobject/gsignal.c:2270:
> signal `changed' is invalid for instance `0x6becc0'
>
> (shp2pgsql-gui:29945): GLib-GObject-WARNING **: IA__g_object_set_valist:
> object class `GtkCellRendererToggle' has no property named `editable'
>
> (shp2pgsql-gui:29945): Gtk-CRITICAL **: gtk_list_store_get_value: assertion
> `column < list_store->n_columns' failed
>
> (shp2pgsql-gui:29945): GLib-GObject-CRITICAL **: g_object_set_property:
> assertion `G_IS_VALUE (value)' failed
>
> (shp2pgsql-gui:29945): GLib-GObject-CRITICAL **: g_value_unset: assertion
> `G_IS_VALUE (value)' failed
>
>
> - Bitrot/regressions compared to 1.5 branch
>
> It looks as if the merge into trunk has removed some of the later changes
> from the 1.5 branch, e.g. take a look at pgui_read_connection(). The version
> in trunk has now regressed to a much older version, and is totally different
> from the logic in the 1.5 branch. Does that mean that there are other
> instances of this from the merge?
>
>
> - Test connection always displays "Connection Succeeded" even when it
> doesn't?!
>
>
> - General code quality issues:
>
> Random bits of code are commented out with no explanation, e.g shp2pgsql-gui
> lines 43-46, and lines 1176-1179. These sections need to either be fixed up
> or removed. Also what does the sqlform query do? It's not obvious and needs
> a comment. Some sections of code are still marked TBD.
>
>
> To summarise, my feeling is that in its current state this code is not ready
> for the prime time, and the maintenance is going upwards due to trunk not
> being based on a recent version of 1.5 branch. I think someone needs to step
> up to fix/maintain this code ASAP or we should simply revert the changes and
> re-synchronise with the 1.5 branch :(
>
>
> ATB,
>
> Mark.
>
> --
> Mark Cave-Ayland - Senior Technical Architect
> PostgreSQL - PostGIS
> Sirius Corporation plc - control through freedom
> http://www.siriusit.co.uk
> t: +44 870 608 0063
>
> Sirius Labs: http://www.siriusit.co.uk/labs
> _______________________________________________
> postgis-devel mailing list
> postgis-devel at postgis.refractions.net
> http://postgis.refractions.net/mailman/listinfo/postgis-devel
>



-- 
Mark Leslie
Geospatial Software Architect
LISAsoft

-------------------------------------------------------------
Ph: +61 2 8570 5000 Fax: +61 2 8570 5099 Mob: +61
Suite 112, Jones Bay Wharf 19-21 Pirrama Rd Pyrmont NSW 2009
-------------------------------------------------------------

LISAsoft is part of the A2end Group of Companies
http://www.ardec.com.au
http://www.lisasoft.com
http://www.terrapages.com