<table><tr><td style="">bruns added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D27152">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D27152#inline-157842">View Inline</a><span style="color: #4b4d51; font-weight: bold;">hallas</span> wrote in <span style="color: #4b4d51; font-weight: bold;">fstabhandling.cpp:374</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I guess the main reason is that the id is a function of a <tt style="background: #ebebeb; font-size: 13px;">FilesystemEntry</tt>, 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 <tt style="background: #ebebeb; font-size: 13px;">FilesystemEntry</tt> class or a static member function of some other class (taking the <tt style="background: #ebebeb; font-size: 13px;">FilesystemEntry</tt> 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.</p>
<p style="padding: 0; margin: 8px;">So long story short :)<br />
I think we should either keep it here or make it a public (static) method somewhere else - <a href="https://phabricator.kde.org/p/bruns/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@bruns</a> what do you think?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">For now just keep it here, I can see now benefit of making it public (even unexported).</p>
<p style="padding: 0; margin: 8px;">And thanks for baring with my nitpicking ;-), this now is pretty much what I hoped for.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R245 Solid</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D27152">https://phabricator.kde.org/D27152</a></div></div><br /><div><strong>To: </strong>hallas, Frameworks, bruns, meven<br /><strong>Cc: </strong>kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns<br /></div>