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