Review Request 124123: Feedback request. Problems ToolView GUI changes and ProblemModel refactoring.

Laszlo Kis-Adam dfighter1985 at gmail.com
Mon Jul 13 02:27:05 UTC 2015 


> On July 3, 2015, 3:41 p.m., Aleix Pol Gonzalez wrote:
> > I'd merge this, then we can start testing.
> > 
> > Please make sure there isn't regressions in our CI.
> 
> Laszlo Kis-Adam wrote:
>     What do you mean by CI?
> 
> Aleix Pol Gonzalez wrote:
>     https://build.kde.org/
> 
> Milian Wolff wrote:
>     run the tests locally and make sure no regressions occur.
> 
> Laszlo Kis-Adam wrote:
>     OK, what procedure should I follow, exactly?
>     How do I use the "CI" and how do I run those tests?
> 
> Aleix Pol Gonzalez wrote:
>     CI is always run, every time you push something.
>     To run the tests, you can go to the build directory and run "ctest ."

I've run the tests for both KDevPlatform and KDevelop.
The same tests fail for both master branch and my own branch.
Some of them simply fail, some actually crashes.

KDevPlatform:
The following tests FAILED:
	 24 - test_sessioncontroller (Failed)

KDevelop:
The following tests FAILED:
	  4 - test_parser (Failed)
	 15 - test_cppassistants (Failed)
	 16 - test_cppcodecompletion (Failed)
	 17 - test_cppcodegen (Failed)
	 18 - test_cmakemanager (Failed)
	 21 - test_gdb (Failed)


- Laszlo


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124123/#review82040
-----------------------------------------------------------


On July 13, 2015, 1:52 a.m., Laszlo Kis-Adam wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124123/
> -----------------------------------------------------------
> 
> (Updated July 13, 2015, 1:52 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> Hi there!
> Here's the bottomline of what I've been doing:
> Please DO note that this is NOT done yet, I'm only submitting it to get some feedback.
> So it should NOT be merged!
> 
> (mostly)GUI changes:
> ====================
> - Moved ProblemModel to shell
> - Reworked the Problems toolview a little. Now it works like this:
> -- ProblemModels are added to ProblemModelSet.
> -- ProblemReporterFactory makes instances of ProblemsView.
> -- ProblemsView takes the models from ProblemModelSet (also subscribes for updates about them, so if one is added or removed it can add/remove their views) and it provides a tabbed widget where the views for them can be added. It creates instances of ProblemTreeView which show the problems in ProblemModel, and adds them to the tabs. Also the tabs shows the number of problems in the ProblemModels.
> -- The toolview will only add actions that are supported by the model (for example: filtering, grouping, reparsing, showing imports. Obviously reparsing doesn't make sense for runtime problem checkers)
> 
> See video:
> https://www.youtube.com/watch?v=K-wcoXcz-GM
> 
> 
> ProblemModel details:
> =====================
> - Broke up ProblemModel into 2 parts
> -- Base ProblemModel that provides the QAbstractItemModel interface for views and can use various ProblemStores to store problems. By default it uses FilteredProblemStore.
> -- ProblemReporterModel is basically the old ProblemModel that grabs problems from DUChain, it's a subclass of ProblemModel.
> - ProblemStore simply stores problems as a list (well technically it stores them in a tree, but it only has 1 level, so it's a list). There's no filtering, no grouping. It's perfect for ProblemReporterModel since it does filtering itself when grabbing the problems from DUChain.
> - FilteredProblemStore DOES filtering, and grouping itself. It stores problems in a tree (ProblemStoreNode subclasses). The tree structure depends on the grouping method, which is implemented with GroupingStrategy subclasses.
> - Moved WatchedDocumentSet and it's subclasses from ProblemModel to ProblemStore, as it is really a detail that the model itself doesn't need, however ProblemStore which stores the problems needs it actually.
> - Created a new Problem class, DetectedProblem. The intent here was to create a class with a clear interface for problems, which ProblemStore can simply store. Then I wanted to eventually replace the "old" Problem class with it. Then I realized that it's not practical because of the "show imports" feature. Which shows the problems from imported contexts. Unfortunately DUChain is the class that knows those, and it's way too much work to get it out from it. Not to mention it doesn't even make sense, since it's really something that logically belongs there. 
> Therefore my current plan now is to just bring them under a common interface so that it will not matter at all. The connection to DUchain will be hidden in the implementation.
> So the work that's left is creating this common interface and then making these two problem classes use it.
> 
> Using this new system is fairly straightforward.
> See the test plugin that I've created to test with:
> https://bitbucket.org/dfighter1985/kdev-problemtest/src
> 
> Especially widget.cpp which instantiates the model, and adds problems, or clears them based on which button is clicked.
> 
> 
> Here's a class diagram about all this:
> http://i.imgur.com/eGWR3sO.jpg
> 
> 
> Diffs
> -----
> 
>   interfaces/CMakeLists.txt 11fa364 
>   interfaces/ilanguagecontroller.h d76284c 
>   interfaces/iproblem.h PRE-CREATION 
>   language/backgroundparser/parsejob.cpp f752207 
>   language/codegen/templateengine_p.h fcb5a06 
>   language/duchain/navigation/problemnavigationcontext.h f19aa18 
>   language/duchain/navigation/problemnavigationcontext.cpp 99e2fd0 
>   language/duchain/navigation/problemnavigationcontext.cpp 99e2fd0 
>   language/duchain/problem.h eb3951a 
>   language/duchain/problem.cpp 8cbf45b 
>   language/duchain/tests/test_duchain.cpp 8d49a24 
>   plugins/problemreporter/CMakeLists.txt 39b3643 
>   plugins/problemreporter/problemhighlighter.h 1df7a79 
>   plugins/problemreporter/problemhighlighter.cpp b4a6ad2 
>   plugins/problemreporter/problemmodel.h 0d7d773 
>   plugins/problemreporter/problemmodel.cpp 9043a24 
>   plugins/problemreporter/problemreportermodel.h PRE-CREATION 
>   plugins/problemreporter/problemreportermodel.cpp PRE-CREATION 
>   plugins/problemreporter/problemreporterplugin.h 5afd8ad 
>   plugins/problemreporter/problemreporterplugin.cpp 5ca0c8d 
>   plugins/problemreporter/problemsview.h PRE-CREATION 
>   plugins/problemreporter/problemsview.cpp PRE-CREATION 
>   plugins/problemreporter/problemtreeview.h b9247d4 
>   plugins/problemreporter/problemtreeview.cpp 1c42034 
>   plugins/problemreporter/watcheddocumentset.h 9dda54d 
>   plugins/problemreporter/watcheddocumentset.cpp 143ee58 
>   shell/CMakeLists.txt cffa161 
>   shell/filteredproblemstore.h PRE-CREATION 
>   shell/filteredproblemstore.cpp PRE-CREATION 
>   shell/languagecontroller.h b9a3d06 
>   shell/languagecontroller.cpp d15a14b 
>   shell/problem.h PRE-CREATION 
>   shell/problem.cpp PRE-CREATION 
>   shell/problemmodel.h PRE-CREATION 
>   shell/problemmodel.cpp PRE-CREATION 
>   shell/problemmodelset.h PRE-CREATION 
>   shell/problemmodelset.cpp PRE-CREATION 
>   shell/problemstore.h PRE-CREATION 
>   shell/problemstore.cpp PRE-CREATION 
>   shell/problemstorenode.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124123/diff/
> 
> 
> Testing
> -------
> 
> See the included video.
> 
> 
> Thanks,
> 
> Laszlo Kis-Adam
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150713/9f593f97/attachment-0001.html>

More information about the KDevelop-devel mailing list