<table><tr><td style="">tcberner 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/D6004" rel="noreferrer">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/D6004#inline-25576" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">graesslin</span> wrote in <span style="color: #4b4d51; font-weight: bold;">CMakeLists.txt:78</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I would keep this information about the contact. We want to be friendly here and it's such a touchy topic that I prefer to be very explicit that we do want to support other solutions.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Ok.</p></div></div><br /><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/D6004#inline-25578" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">graesslin</span> wrote in <span style="color: #4b4d51; font-weight: bold;">CMakeLists.txt:81-87</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I would prefer if we only search for one of the two. What I want to not have is a situation where we get false positive warnings about missing runtime components. E.g. if your distro is using logind there is just no point in looking for ConsoleKit and it would now produce a false positive runtime warning. The same applies the other way around.</p>

<p style="padding: 0; margin: 8px;">Maybe we could work with add_feature_info?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Ok, I'll adapt it.</p></div></div><br /><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/D6004#inline-25583" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">graesslin</span> wrote in <span style="color: #4b4d51; font-weight: bold;">abstractlocker.cpp:62</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">cant we make the bash script just do what it is supposed to do?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I'm not sure, if we can predictably get the name of the correct "broken" session from within the script -- of course, the script could just unlock all the sessions by iterating over them, but that seems rude. Maybe the error message <tt style="background: #ebebeb; font-size: 13px;">text</tt> could be modified to contain correct session name:</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">"# ck-unlock-session [the current session name]\n\n"</pre></div></div></div><br /><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/D6004#inline-25582" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">graesslin</span> wrote in <span style="color: #4b4d51; font-weight: bold;">abstractlocker.cpp:67-70</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I would scrap that part. It's too technical and doesn't help the user.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I think seeing this message annoys the users. So pointing them to possible solutions for future failures is a good thing, even when it might be a bit technical.</p></div></div><br /><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/D6004#inline-25584" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">graesslin</span> wrote in <span style="color: #4b4d51; font-weight: bold;">ck-unlock-session.cmake:17</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Is this asking for the password in some way? Or do we now install a script which allows to bypass authentication? With logind one can configure the system to always require a password.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">It does not ask for a password, but by default, only root should be able to do it.</p></div></div><br /><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/D6004#inline-25585" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">graesslin</span> wrote in <span style="color: #4b4d51; font-weight: bold;">FindConsoleKit.cmake:28</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">just wondering: why dbus-send and not qdbus? qdbus would be a binary our session depends on anyway.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">No reason, just didn't think of it. I will try to change it to qdbus.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R133 KScreenLocker</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D6004" rel="noreferrer">https://phabricator.kde.org/D6004</a></div></div><br /><div><strong>To: </strong>tcberner, FreeBSD, graesslin, Plasma<br /><strong>Cc: </strong>erichameleers, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas<br /></div>