search

strk / qgis_pgis_topoedit

PostGIS topology editor plugin for qgis
GNU General Public License v3.0
18 stars 9 forks source link

Migrated to QGIS 3 #7

Closed dafvid closed 4 years ago

dafvid commented 5 years ago

This is my first commit on trying to migrate this plugin to QGIS 3. Please comment.

I'm on windows and instead of trying to get make to work I instead created a makefile.py that does the same thing i python with some argument hacking.

strk commented 5 years ago

Thanks for this ! Unfortunately there are too many changes for me to review quickly. Can you reduce them to the bare minimum ? Also don't put in unrelated changes (style, makefile.py...)

dafvid commented 5 years ago

Sorry about the mess. This should be a bare minimum change.

dafvid commented 5 years ago

Can you please give me a link to the recommendation for using separate branches for supporting qgis pre and post 3.0 ?

This post on the qgis github wiki links to this post quoted below. Indirectly official I suppose.

Unless you absolutely want to make your code run on both API 2 and 3 (which might be possible) I strongly suggest to create a branch or your current version called qgis2, API2 or legacy or whatever you want to call it. From now on this branch will be responsible for all your future (probably mainly bugfixes) releases for the 2.x series of QGIS.

strk commented 5 years ago

On Tue, May 07, 2019 at 11:40:48PM -0700, David Wahlund wrote:

Passing just (str(layer_id)) to cur.execute() will pass the str as an array of char, I think. But adding a comma as so (str(layer_id),) passes a single item tuple instead.

If this is the case this is a bad bug and should be fixed independently of QGIS compatibility, as it would also affect earlier releases, right ?

I guess it means the plugin would fail to delete orphaned topogeometries or WORST could delete non-orphaned ones.

This would happen when the layer_id has more than one digits, is that what you're saying ?

Would you be up for testing this and file a separate PR to fix ?

strk commented 5 years ago

This post on the qgis github wiki links to this post quoted below. Indirectly official I suppose.

Thanks! I've created a qgis2 branch, as recommended in that post.