https://bugs.kde.org/show_bug.cgi?id=254779
--- Comment #2 from Rolf Eike Beer <k...@opensource.sf-tec.de> --- The patch looks good in principle, however I have some nitpicks: Optional: -if you have a KDE account I would welcome if you could put the patch into Reviewboard instead, referencing this bug id -if at all possible I would like to have this patch rebased on the frameworks branch as Applications/16.08 is feature and string frozen and I hope we will get the frameworks branch ready before 16.12, i.e. there will likely be no further releases from the master branch Mandatory: -as you are using git to create the patch, please do a local "git commit" and "git format-patch -1" to generate the patch, containing your correct author information as well as a "BUG:254779" and "REVIEW:…" line -m_expiry is used but not declared -the initializer list for KGpgSortFilterProxyModel misses some linebreaks, see e.g. SearchResult for an example -KGpgSortFilterProxyModel is a bit too general as class name, as this filter is about search results I would suggest something like KGpgSearchResultFilterModel -since this class can only properly work with a KGpgSearchResultModel as input model I would add this as a required argument in the constructor, so no invalid state can accidentially happen -the logic in filterAcceptsRow() should be rewritten to first check the parent filterAcceptsRow(). If that returns false, just return false, otherwise don't bother to check it in the rest of the function. Since both branches of the final "if" end returning the same value move that code out of the if. The coding style for the else is also wrong, it should be "} else {" in one line, although the else can go entirely away now that the if-branch became empty. -I obviously fail the wtf-test with my model design, so I suggest you remove your comment in this commit and come up with one that has a better code design Not that mandatory: -even more, since KGpgSearchResultModel is now never used "alone" I would make the former model an implementation detail of the proxy model (i.e. private) and have the new class as KGpgSearchResultModel -- You are receiving this mail because: You are watching all bug changes.