D27152: Introduce FilesystemEntry class

[David Hallas]

hallas added inline comments.

INLINE COMMENTS

> bruns wrote in fstabhandling.cpp:374
> Why is the id() a property of the entry now?
> 
> - it does not correspond to anything from the fstab/mtab
> - it is only used here to generate the key for the cache
> 
> Please move the id generation back here, eventually changing it from `_k_deviceNameForMountpoint(const QString &source, const QString &fstype,const QString &mountpoint)` to `_k_deviceIdForFsentry(const FilesystemEntry&)`

I guess the main reason is that the id is a function of a `FilesystemEntry`, meaning that the id is computed from the contents of it. You are absolutely right that it is not picked up directly from the fstab/mtab file, but it is computed from the contents. I think it should either be a member function of the `FilesystemEntry` class or a static member function of some other class (taking the `FilesystemEntry` as an input paramter) - it at least has to be in a place where it is possible to write a unit test for it's functionality. especially since the id() is used as part of the public API. Therefore I don't think we should make this an internal function in fstabhandling. Another reason is that, we might need the id in other places then the fstabhandling, at least we need it for the rest of the fstab handling refactor I posted in the other review.

So long story short :)
I think we should either keep it here or make it a public (static) method somewhere else - @bruns what do you think?

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/20200309/c51c472e/attachment.html>