push almost all c++ code to a qml plugin for use by Elisa and the tests
ClosedPublic

Authored by mgallien on Mar 27 2018, 6:08 AM.

Details

Summary

push almost all c++ code to a qml plugin for use by Elisa and the tests

Test Plan

Elisa main application still works. More work is needed to avoid compiling n times the source code for the different automatic tests for example.

Diff Detail

Repository
R255 Elisa
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mgallien requested review of this revision.Mar 27 2018, 6:08 AM
mgallien created this revision.

I'm not sure that I understand all of that, so I'm probably not the right one to review it.
For my own understanding: The Qml Plugin gets automatically loaded when we start the qml engine? Because I don't see it anywhere getting loaded in the main.cpp file.
Personally I would prefer that only the type registration is moved to the plugin and that elisaapplications.cpp etc. would still get created in the main.cpp file. That would keep the code more readable

mgallien updated this revision to Diff 32501.Apr 18 2018, 6:48 PM
  • behavior of ElisaApplication has changed: add new enqueue signal in test
  • fix execution of the automatic tests

rebase on top of master

I'm not sure that I understand all of that, so I'm probably not the right one to review it.

It is the standard way to have code that can be loaded by a Qml engine. This is the way Kirigami works for example.

For my own understanding: The Qml Plugin gets automatically loaded when we start the qml engine? Because I don't see it anywhere getting loaded in the main.cpp file.

Yes, the import lines will do exactly that.

Personally I would prefer that only the type registration is moved to the plugin and that elisaapplications.cpp etc. would still get created in the main.cpp file. That would keep the code more readable

I will have a look at what I can do here.

mgallien updated this revision to Diff 32625.Apr 20 2018, 7:47 AM
  • fix one warning generated by clang through QtCreator

In fact, the goal is to be able to run automatic tests of qml components that may require the c++ code and the global variable elisa.
The other important thing is to reduce compilation time by not compiling n times each cpp files.

In fact, the goal is to be able to run automatic tests of qml components that may require the c++ code and the global variable elisa.
The other important thing is to reduce compilation time by not compiling n times each cpp files.

That would be cool indeed, but I still would prefer if this can be done without pushing elisapplication to the plugin loader. The other projects I looked at use the plugin loader to pass the types to qml, but I haven't seen anyone relying on the plugin loader for basically the whole application logic.
Is it be possible to create another plugin just for the tests?

mgallien planned changes to this revision.Apr 29 2018, 7:48 AM

I plan to do the following steps:

  • put all c++ code in a shared library for use by the applications, the qml plugin and the automatic tests ;
  • put the creation of elisa main variable in elisa application ;
  • ensure the qml test that needs c++ code still works with a locally created instance of elisa application.

This way, we should be able to reduce a lot compilation time. We should be able to have a usable qml plugin for the automatic tests. We should also keep creating the ElisaApplication instance in the main.cpp file.
Does it sound good for you ?

I plan to do the following steps:

  • put all c++ code in a shared library for use by the applications, the qml plugin and the automatic tests ;
  • put the creation of elisa main variable in elisa application ;
  • ensure the qml test that needs c++ code still works with a locally created instance of elisa application.

    This way, we should be able to reduce a lot compilation time. We should be able to have a usable qml plugin for the automatic tests. We should also keep creating the ElisaApplication instance in the main.cpp file. Does it sound good for you ?

That sounds like a good plan, thanks!

mgallien updated this revision to Diff 33305.Apr 29 2018, 7:33 PM
  • elisaLib is a library with most c++ code and we also have got a qml plugin

Works flawlessly! One question below

src/main.cpp
134

Is going via a qml context property really necessary? There should be a way to pass the arguments directly

mgallien planned changes to this revision.May 1 2018, 7:36 AM
mgallien added inline comments.
src/main.cpp
134

I should probably pass the arguments directly to the myApp object.

mgallien updated this revision to Diff 33405.May 1 2018, 4:21 PM
  • behavior of ElisaApplication has changed: add new enqueue signal in test
  • fix one warning generated by clang through QtCreator
  • elisaLib is a library with most c++ code and we also have got a qml plugin
  • pass arguments directly without going to qml
astippich accepted this revision.May 1 2018, 4:34 PM
This revision is now accepted and ready to land.May 1 2018, 4:34 PM
This revision was automatically updated to reflect the committed changes.