hallas added a comment.
@bruns - thanks a lot for the feedback, I have updated the patch with your suggestions. INLINE COMMENTS > bruns wrote in filesystem_entry.cpp:77 > Why don't you just keep the string? It was merely meant as an optimization, I don't know if it makes sense - I'll remove it. > bruns wrote in fstabdevice.cpp:41 > Ircs, raw pointers ... you newer know who owns them ... It is a pointer to an > entry in a QHash, it may be deleted/moved when you don't expect it. > > Unfortunately, std::optional is C++17 only. > > Return it by value, return a default constructed instance if no found, and > compare `fsEntry.device() != m_device` Good point :) I have changed this as you suggested, but I have added a method to `FilesystemEntry` called `isValid` to check if it is valid > bruns wrote in fstabhandling.cpp:264 > this looks wrong ... Yes definitely :) I have removed the line > bruns wrote in fstabhandling.cpp:316 > this is bad, bad, bad, ... Fixed REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D27152 To: hallas, #frameworks, bruns, meven Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
