D27152: Introduce FilesystemEntry class

Méven Car noreply at phabricator.kde.org
Wed Feb 5 07:51:37 GMT 2020


meven added inline comments.

INLINE COMMENTS

> hallas wrote in filesystem_entry.cpp:47
> Yeah I agree ;) But I would really like for this class to be used more generically so having a list of filesystems here would probably make it hard for that. We could also have two methods, one similar to this one given the "raw" filesystem type, and one that would give an enum entry from a so called "known" list, but would return "unknown" for a filesystem type not in the list, then you would have access to both? What do you think?

This sound sane to me.
Ideally we would store only the string in case of an unknown fs or the enum but not both for a same entry, the choice could be done in ctor.

> hallas wrote in filesystem_entry.h:88
> No, I only put them here for completeness sake. Basically I sat down with the fstab man page and the header file documenting the mntent structure and based this class around that, but if this code should be exported an made generally available then we should probably add these, what do you think? Should I remove them now, and then we can always add them if needed?

Just leave a leave a comment as to why we want to keep those.

> fstabhandling.cpp:388
> +    QStringList mountpoints;
> +    for (const auto& dev : globalFstabCache->localData().m_mtabCache) {
> +        if (!dev.mountPath().isEmpty()) {

Detaches m_mtabCache, just qAsConst or temporary const variable

REPOSITORY
  R245 Solid

REVISION DETAIL
  https://phabricator.kde.org/D27152

To: hallas, #frameworks, bruns, meven
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200205/beb2e8e2/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list