[Development] Some Qt3D feedback
steveire at gmail.com
Mon Jun 8 01:11:23 CEST 2015
Congrats to Paul, Sean and others working on getting this module in a
releasable state for Qt 5.5!
I have not reviewed the code, but I found some items to raise:
1) The include/Qt3DCore/Window file doesn't have a Q prefix.
as every other header does. Should probably be Qt3DWindow.
2) A private header is included in a public header:
include/Qt3DCore$ grep private/ *.h
This is concerning - Don't we have a unit test preventing that?
3) The cmake unit tests don't pass.
It is easily fixable, but does this mean that the cmake tests are not run
for this module? That is concerning.
Are unit tests run for this module in CI at all?
4) Private dependencies
git grep -w -e QT --and -e private
shows a bunch of content. Shouldn't they be added to QT_PRIVATE instead?
5) Qt3D namespace
This is the first time that all classes in a library are in a namespace.
Previously only enums (in various modules) and free functions (in
QtConcurrent) have been put in namespaces.
In QtConcurrent, the module name also appears in the header file, but that
is not followed by Qt3D libraries.
Given that Qt has never put classes in a namespace like this, is there
something to be consistent about here?
6) QParameter is a very generic name
I realise it is in a namespace, but still...
Qt3DParamter might be better *and* more consistent. Similar applies to other
7) Unneeded Q_DECLARE_METATYPE
Using the macro for QObject derived types is not needed. I saw
and similar for QParameter and then stopped searching.
8) Docs in headers.
I saw one method documented in qparameter.h instead of in the cpp and I
didn't look for more.
More information about the Development