Review Request: ProjectSelection class to select one of the opened projects

[Daniel Calviño Sánchez]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1534/
-----------------------------------------------------------

Review request for KDevelop.


Summary
-------

I have refactored the KSelectAction used in xTest plugin to select a project and extracted its functionality (plus some features not present in the original code) to a class named ProjectSelection, which is a KSelectAction that contains an item for each opened project. I've refactored KDevPlatform xTest plugin to use that class, and I'll also use it in KDevelop Coverage plugin (that's why I extracted the functionality to a self contained class).

However, I have some questions about KDevPlatform code organization and style.

First of all, where should I put the new ProjectSelection class? In this patch I put it in "kdevplatform/veritas" just because I extracted the class from veritas code, but it doesn't really depend on veritas framework. In fact, KDevelop Coverage plugin (the one I'm going to use it in) is no longer part of veritas/xUnit and it doesn't even link to it. So, is there a more suitable place for that class?

Next, should I use d-pointers for ProjectSelection class? Right now it has several private attributes, but I don't know if there is any ABI compatibility restriction to follow.

Internally, ProjectSelection connects to IProjectController signals, and it gets it from ICore. If there is no Core, it doesn't work (and, in fact, it crashes). In normal use there will always be a Core (or I think so), but in unit tests this forces the tests to initialize a Shell (AutoTestShell) and Core whenever a ProjectSelection is created.

This is inevitable in ProjectSelection test itself, but can be annoying when testing something that creates a ProjectSelection (like veritas framework), as it shows a main window which isn't used for anything. So I made a check in ProjectSelection constructor to not to connect any slot if there is no Core. However, I'm not really satisfied with this approach, as this is modifying the code to make tests more comfortable. So, should I keep the check or remove it and fix the tests (causing the annoyance of the unneeded main window)? Is there any other solution?

Finally, a ProjectSelection can filter which projects are enabled to be selected and which not using a ProjectFilter subclass object. ProjectFilter is an abstract base class to be extended with concrete filter behavior. Should it be renamed to IProjectFilter (although it is not a pure interface)? Moreover, right now it is an internal class of ProjectSelection. Should I move it out? If yes, to its own file or in projectselection.h itself but out of ProjectSelection class?

Thank you for your attention.


Diffs
-----

  /trunk/KDE/kdevplatform/veritas/CMakeLists.txt 1019996 
  /trunk/KDE/kdevplatform/veritas/internal/runnerwindow.h 1019996 
  /trunk/KDE/kdevplatform/veritas/internal/runnerwindow.cpp 1019996 
  /trunk/KDE/kdevplatform/veritas/projectselection.h PRE-CREATION 
  /trunk/KDE/kdevplatform/veritas/projectselection.cpp PRE-CREATION 
  /trunk/KDE/kdevplatform/veritas/testrunner.h 1019996 
  /trunk/KDE/kdevplatform/veritas/testrunner.cpp 1019996 
  /trunk/KDE/kdevplatform/veritas/tests/CMakeLists.txt 1020213 
  /trunk/KDE/kdevplatform/veritas/tests/projectselectiontest.h PRE-CREATION 
  /trunk/KDE/kdevplatform/veritas/tests/projectselectiontest.cpp PRE-CREATION 

Diff: http://reviewboard.kde.org/r/1534/diff


Testing
-------


Thanks,

Daniel