[Development] Some Qt3D feedback

[Stephen Kelly]

On Tue, Jun 9, 2015 at 11:01 AM, Sean Harmer <sean.harmer at kdab.com> wrote:
> On Monday 08 Jun 2015 14:18:33 Sean Harmer wrote:
>>
>> On Monday 08 Jun 2015 01:11:23 Stephen Kelly wrote:
>> > 2) A private header is included in a public header:
>> >  include/Qt3DCore$ grep private/ *.h
>> >  qaspectjobmanager.h:#include <Qt3DCore/private/qt3dcore_global_p.h>
>> >
>> > This is concerning - Don't we have a unit test preventing that?
>>
>> I can't find that. Is that from the 5.5 branch?
>
> Aha, found it thanks to the cmake unit test ;) Fix at

Yep, that's how I found it too. I'm confused that the grep I pasted
did not lead you to it though. The grep shows it to me on 5.5, 5.5.0
and dev branches. Maybe you didn't know the directory to run the grep
in? But even then, the filename should be enough to lead you to the
private/ include...

Anyway, good you found it.

> https://codereview.qt-project.org/#/c/113953/
>
> It's failing due to the use of namespace in Qt3D. Does the cmake unit test
> infrastructure support namespaces? Any hints on how to fix or to add support
> for namespaces.

I just commented on the review. For reference, you need to set a
${ClassName}_NAMESPACE variable to the namespace to use for a class
under test.

> I note that the cmake tests for other modules pass with
> namespaced Qt builds but I guess that may be using a different mechanism.

What you're doing with namespaces here is different to 'a namespaced
Qt', so yep, different mechanism.

Thanks,

Steve.

On Tue, Jun 9, 2015 at 11:01 AM, Sean Harmer <sean.harmer at kdab.com> wrote:
> On Monday 08 Jun 2015 14:18:33 Sean Harmer wrote:
>>
>> On Monday 08 Jun 2015 01:11:23 Stephen Kelly wrote:
>> > 2) A private header is included in a public header:
>> >  include/Qt3DCore$ grep private/ *.h
>> >  qaspectjobmanager.h:#include <Qt3DCore/private/qt3dcore_global_p.h>
>> >
>> > This is concerning - Don't we have a unit test preventing that?
>>
>> I can't find that. Is that from the 5.5 branch?
>
> Aha, found it thanks to the cmake unit test ;) Fix at
>
> https://codereview.qt-project.org/#/c/113952/
>
>> > 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?
>>
>> Hmm I thought they were, but obviously not. I'm looking at making the test
>> work now.
>
> Partial fix at
>
> https://codereview.qt-project.org/#/c/113953/
>
> It's failing due to the use of namespace in Qt3D. Does the cmake unit test
> infrastructure support namespaces? Any hints on how to fix or to add support
> for namespaces. I note that the cmake tests for other modules pass with
> namespaced Qt builds but I guess that may be using a different mechanism.
>
> Cheers,
>
> Sean
> --
> Dr Sean Harmer | sean.harmer at kdab.com | Managing Director UK
> KDAB (UK) Ltd, a KDAB Group company
> Tel. +44 (0)1625 809908; Sweden (HQ) +46-563-540090
> Mobile: +44 (0)7545 140604
> KDAB - Qt Experts