[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: Review Request 120839: Fix solid-device-automounter failing to automount unknown devices
From: "Christoph Feck" <christoph () maxiom ! de>
Date: 2014-10-29 16:17:31
Message-ID: 20141029161731.23655.19607 () probe ! kde ! org
[Download RAW message or body]
--===============3735689821977386548==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
> On Oct. 29, 2014, 12:02 p.m., Helio Castro wrote:
> > Ok, way better than the previous one :-)
>
> Frank Schütte wrote:
> Thanks for your approval. Now who is going to commit my patch? I don't have write access to \
> the repository, as far as I know.
> Christoph Feck wrote:
> me. I am just not sure to which branch. When I commit to master, it will only available in \
> 5.x releases.
> Does anyone think it should be in 4.14.3 release?
>
> Thomas Lübking wrote:
> I don't use automounting, but since enbaling automount of unknown devices currently breaks \
> *all* automounting, I'd say this should be fixed in KDE 4 as well.
> Frank Schütte wrote:
> I would love to see this patch in KDE 4, because I rely on automounting in my local setup (I \
> administer about 1000 computers in a school environment).
... wenn es doch für die Kinder ist, gerne ;)
I committed it to 4.14 branch. Thanks for the work, Frank, I 'urge' you to continue finding \
issues that help KDE in such large deployments.
- Christoph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120839/#review69425
-----------------------------------------------------------
On Oct. 29, 2014, 1:56 p.m., Frank Schütte wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120839/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2014, 1:56 p.m.)
>
>
> Review request for kdelibs, Solid, Christoph Feck, and Helio Castro.
>
>
> Bugs: 243046 and 261376
> http://bugs.kde.org/show_bug.cgi?id=243046
> http://bugs.kde.org/show_bug.cgi?id=261376
>
>
> Repository: kde-runtime
>
>
> Description
> -------
>
> Hi Christoph,
> solid-device-automounter fails to automount unknown devices, even if it is checked,
> because in kded/DeviceAutomounter.cpp it fails to evaluate shouldAutomountDevice.
> So this patch corrects the logic to at first evaluate shouldAutomountDevice.
> Inside this evaluation automountUnknownDevices is evaluated correctly.
> Please review this patch. Christoph Feck urged me to submit this patch.
> It took me quite a while to figure it out, though.
> Bye,
> Frank
>
>
> Diffs
> -----
>
> solid-device-automounter/kded/DeviceAutomounter.cpp 14b4e87
> solid-device-automounter/lib/AutomounterSettings.cpp 2b3e6be
>
> Diff: https://git.reviewboard.kde.org/r/120839/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Frank Schütte
>
>
--===============3735689821977386548==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; \
border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/120839/">https://git.reviewboard.kde.org/r/120839/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 29th, 2014, 12:02 p.m. UTC, <b>Helio Castro</b> \
wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; \
white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Ok, way better than the previous \
one :-)</p></pre> </blockquote>
<p>On October 29th, 2014, 1:24 p.m. UTC, <b>Frank Schütte</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; \
white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Thanks for your approval. Now who \
is going to commit my patch? I don't have write access to the repository, as far as I \
know.</p></pre> </blockquote>
<p>On October 29th, 2014, 1:32 p.m. UTC, <b>Christoph Feck</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; \
white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">me. I am just not sure to which \
branch. When I commit to master, it will only available in 5.x releases.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Does anyone \
think it should be in 4.14.3 release?</p></pre> </blockquote>
<p>On October 29th, 2014, 1:42 p.m. UTC, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; \
white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't use automounting, but \
since enbaling automount of unknown devices currently breaks <em style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">all</em> \
automounting, I'd say this should be fixed in KDE 4 as well.</p></pre> </blockquote>
<p>On October 29th, 2014, 2:11 p.m. UTC, <b>Frank Schütte</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; \
white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">I would love to see this patch in \
KDE 4, because I rely on automounting in my local setup (I administer about 1000 computers in a \
school environment).</p></pre> </blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; \
white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">... wenn es doch für die Kinder \
ist, gerne ;)</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I committed it to 4.14 branch. Thanks for the work, Frank, I \
'urge' you to continue finding issues that help KDE in such large deployments.</p></pre> <br />
<p>- Christoph</p>
<br />
<p>On October 29th, 2014, 1:56 p.m. UTC, Frank Schütte wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px \
#888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for kdelibs, Solid, Christoph Feck, and Helio Castro.</div>
<div>By Frank Schütte.</div>
<p style="color: grey;"><i>Updated Oct. 29, 2014, 1:56 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=243046">243046</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=261376">261376</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kde-runtime
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px \
solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Hi Christoph, \
solid-device-automounter fails to automount unknown devices, even if it is checked, because in \
kded/DeviceAutomounter.cpp it fails to evaluate shouldAutomountDevice. So this patch corrects \
the logic to at first evaluate shouldAutomountDevice. Inside this evaluation \
automountUnknownDevices is evaluated correctly. Please review this patch. Christoph Feck urged \
me to submit this patch. It took me quite a while to figure it out, though.
Bye,
Frank</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>solid-device-automounter/kded/DeviceAutomounter.cpp <span style="color: \
grey">(14b4e87)</span></li>
<li>solid-device-automounter/lib/AutomounterSettings.cpp <span style="color: \
grey">(2b3e6be)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120839/diff/" style="margin-left: 3em;">View \
Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============3735689821977386548==--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic