ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed.
INLINE COMMENTS > mlaurent wrote in kfileplacesitem.cpp:202 > This one is for avoiding duplicate code as we create in this patch isHidden > method. That was kind of my point, isHidden could have been introduced on its own with the changes needed to remove the duplication in the same commit. > ervin wrote in kfileplacesmodel.cpp:62-74 > Just make it a const global static then... No need to recreate this hash > several times. Why the intermediate struct? Could be directly the hash as a global static, should make it easier to use you wouldn't need the function below and the extra indirection. > ervin wrote in kfileplacesmodel.h:55 > Better have the curly brace on the same line than the enum name to be > consistent with the other enum just on top. > > Also the enum itself needs to carry over the changes which have been applied > to previous commits. You missed a bit, it looks like it wasn't applied on top of the latest iteration from the other patches the GroupType enum gained some values for its entries. REVISION DETAIL https://phabricator.kde.org/D8367 To: mlaurent, renatoo, ngraham, franckarrecot, ervin Cc: ngraham, mlaurent, #frameworks
