Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove V2 and NG suffixes #11

Open
m-kuhn opened this Issue Dec 1, 2015 · 7 comments

Comments

Projects
None yet
5 participants
@m-kuhn
Copy link
Member

commented Dec 1, 2015

We currently have a lot of these, previous versions should be deprecated or already removed even.

I am not completely convinced that it's a good idea since expected name collisions may produce hard to find errors when updating plugins.

@nyalldawson

This comment has been minimized.

Copy link
Contributor

commented Dec 1, 2015

I'm also not sure that removing ALL the non - v2 classes is necessarily a good idea. For example, while we now have QgsPointV2 I don't think this should replace all uses of QgsPoint. QgsPointV2 is a lot larger/more complex and I think there's still a need for a simple point with just x & y class.

QgsPoint could do with a lot of cleaning though.

While there's some easy decisions (removal of old label classes for instance), basically this would need to be considered carefully on a class by class basis.

@vmora

This comment has been minimized.

Copy link

commented Jan 6, 2017

I realize the need for a per-class reflexion, but renaming or merging the old class in the case of QgsPoint participate to improve readability IMO.

Concerning the name collisions in plugins, I don't see how it is causing 'harder' to find errors. @m-kuhn Are you thinking about c++ plugins or python ones ?

Overall +1 for the proposal, doing without is opening the door to v3 and ngv2 suffixes.

@pcav

This comment has been minimized.

Copy link
Member

commented Jan 6, 2017

Agreed, +1

@m-kuhn

This comment has been minimized.

Copy link
Member Author

commented Oct 23, 2017

Status quo:

get_subclasses(QgsAbstractGeometry)
<class 'qgis._core.QgsAbstractGeometry'>
<class 'qgis._core.QgsCurve'>
<class 'qgis._core.QgsCircularString'>
<class 'qgis._core.QgsCompoundCurve'>
<class 'qgis._core.QgsLineString'>
<class 'qgis._core.QgsSurface'>
<class 'qgis._core.QgsCurvePolygon'>
<class 'qgis._core.QgsPolygonV2'>
<class 'qgis._core.QgsTriangle'>
<class 'qgis._core.QgsPoint'>
<class 'qgis._core.QgsGeometryCollection'>
<class 'qgis._core.QgsMultiCurve'>
<class 'qgis._core.QgsMultiLineString'>
<class 'qgis._core.QgsMultiPointV2'>
<class 'qgis._core.QgsMultiSurface'>
<class 'qgis._core.QgsMultiPolygonV2'>

Can we make this a requirement for freeze? I really wouldn't want this inconsistency to persist during the lifetime of QGIS 3.0

@NathanW2

This comment has been minimized.

Copy link
Member

commented Oct 23, 2017

@nyalldawson

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2017

Can we make this a requirement for freeze? I really wouldn't want this inconsistency to persist during the lifetime of QGIS 3.0

Just to clarify - I don't consider feature freeze to equal api freeze. I think final release = api freeze, no sooner.

@m-kuhn

This comment has been minimized.

Copy link
Member Author

commented Oct 23, 2017

Ok, but can we have an API freeze date prior to the final freeze still? E.g. a change like this should not land the last day before release.

Maybe 2 weeks (during which any further API change will require a very good explanation)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.