D27152: Introduce FilesystemEntry class
David Hallas
noreply at phabricator.kde.org
Wed Mar 4 06:10:00 GMT 2020
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200304/53cd6122/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list