<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/124279/">https://git.reviewboard.kde.org/r/124279/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/124279/diff/1/?file=383513#file383513line189" style="color: black; font-weight: bold; text-decoration: underline;">libs/editor/connectiondetaileditor.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void ConnectionDetailEditor::initEditor()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">189</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">watcher</span><span class="o">-></span><span class="n">setProperty</span><span class="p">(</span><span class="s">"setting-name"</span><span class="p">,</span> <span class="n">settingName</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<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;">Why not use camel case in this property's name?</p></pre>
</div>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/124279/diff/1/?file=383513#file383513line425" style="color: black; font-weight: bold; text-decoration: underline;">libs/editor/connectiondetaileditor.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">NMVariantMapMap ConnectionDetailEditor::setting()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">419</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">SettingWidget</span> <span class="o">*</span> <span class="n">widget</span> <span class="o">=</span> <span class="k">static_cast</span><span class="o"><</span><span class="n">SettingWidget</span><span class="o">*></span><span class="p">(</span><span class="n">m_ui</span><span class="o">-></span><span class="n">tabWidget</span><span class="o">-></span><span class="n">widget</span><span class="p">(</span><span class="n">i</span><span class="p">));</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<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;">Nitpick: two different code styles for pointers here.</p></pre>
</div>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/124279/diff/1/?file=383526#file383526line43" style="color: black; font-weight: bold; text-decoration: underline;">libs/editor/widgets/settingwidget.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">43</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">virtual</span> <span class="kt">void</span> <span class="nf">readSecrets</span><span class="p">();</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">43</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">virtual</span> <span class="kt">void</span> <span class="nf">loadSecrets</span><span class="p">(</span><span class="k">const</span> <span class="n">NetworkManager</span><span class="o">::</span><span class="n">Setting</span><span class="o">::</span><span class="n">Ptr</span> <span class="o">&</span><span class="n">setting</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<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;">From API point of view since now there is a loadScrets() method it should be the only method used to load secrets all over the place. In this patch you kept loadConfig() loading secrets too, which is code duplication.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Another alternative is keeping the code as it is now and just add a QProgressDialog [1] to indicate to the user that secrets are being loaded. QProgressDialog only shows up when it calculates (internally) that it will be visible for more than 4 seconds (default value, it is adjustable). This way the dialog appears only when loading secrets is taking too much time. We can make dialog modal to prevent user from saving the connection configuration.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">[1] http://doc.qt.io/qt-5/qprogressdialog.html</p></pre>
</div>
</div>
<br />
<p>- Lamarque Souza</p>
<br />
<p>On July 7th, 2015, 8:15 a.m. UTC, Jan Grulich 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 Network Management and Lamarque Souza.</div>
<div>By Jan Grulich.</div>
<p style="color: grey;"><i>Updated July 7, 2015, 8:15 a.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=349002">349002</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-nm
</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;">Previously the UI of the editor was completely loaded only when we got respond from NM with or without secrets. Problem is that in some cases, eg. when our kded module is not loaded, the request will fail and we need to wait until the request times out, which leads to a problem described in BUG 349002. I changed this behaviour to make the UI load immediately and load secrets additionaly in case we get respond from NM, otherwise the UI will be loaded without secrets. The only problem I can think of is when secrets won't be loaded and the user will save its connection without secrets, but in most cases it shouldn't let him to press the OK button because of validation.</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>editor/connectioneditor.h <span style="color: grey">(1776a36)</span></li>
<li>libs/editor/connectiondetaileditor.cpp <span style="color: grey">(f84a197)</span></li>
<li>libs/editor/settings/cdmawidget.h <span style="color: grey">(99b6b43)</span></li>
<li>libs/editor/settings/cdmawidget.cpp <span style="color: grey">(566670c)</span></li>
<li>libs/editor/settings/gsmwidget.h <span style="color: grey">(ce11371)</span></li>
<li>libs/editor/settings/gsmwidget.cpp <span style="color: grey">(fff73d8)</span></li>
<li>libs/editor/settings/pppoewidget.h <span style="color: grey">(45df517)</span></li>
<li>libs/editor/settings/pppoewidget.cpp <span style="color: grey">(42e117e)</span></li>
<li>libs/editor/settings/security802-1x.h <span style="color: grey">(6f79fc7)</span></li>
<li>libs/editor/settings/security802-1x.cpp <span style="color: grey">(0ecc333)</span></li>
<li>libs/editor/settings/wifisecurity.h <span style="color: grey">(fd2e006)</span></li>
<li>libs/editor/settings/wifisecurity.cpp <span style="color: grey">(71d700c)</span></li>
<li>libs/editor/settings/wiredsecurity.h <span style="color: grey">(cfe7f78)</span></li>
<li>libs/editor/settings/wiredsecurity.cpp <span style="color: grey">(616aa70)</span></li>
<li>libs/editor/widgets/settingwidget.h <span style="color: grey">(8d07b73)</span></li>
<li>libs/editor/widgets/settingwidget.cpp <span style="color: grey">(146113d)</span></li>
<li>vpn/l2tp/l2tpwidget.h <span style="color: grey">(eed2b24)</span></li>
<li>vpn/l2tp/l2tpwidget.cpp <span style="color: grey">(fa6118b)</span></li>
<li>vpn/openswan/openswanwidget.h <span style="color: grey">(ce5a04f)</span></li>
<li>vpn/openswan/openswanwidget.cpp <span style="color: grey">(ca98e6d)</span></li>
<li>vpn/openvpn/openvpnwidget.h <span style="color: grey">(d7afcdb)</span></li>
<li>vpn/openvpn/openvpnwidget.cpp <span style="color: grey">(18b0b70)</span></li>
<li>vpn/pptp/pptpwidget.h <span style="color: grey">(1645a92)</span></li>
<li>vpn/pptp/pptpwidget.cpp <span style="color: grey">(a540c81)</span></li>
<li>vpn/ssh/sshwidget.h <span style="color: grey">(f05caf8)</span></li>
<li>vpn/ssh/sshwidget.cpp <span style="color: grey">(5d077c7)</span></li>
<li>vpn/sstp/sstpwidget.h <span style="color: grey">(1c60252)</span></li>
<li>vpn/sstp/sstpwidget.cpp <span style="color: grey">(67c54ca)</span></li>
<li>vpn/strongswan/strongswanwidget.h <span style="color: grey">(8101bb7)</span></li>
<li>vpn/strongswan/strongswanwidget.cpp <span style="color: grey">(88a2ede)</span></li>
<li>vpn/vpnc/vpncwidget.h <span style="color: grey">(69beb97)</span></li>
<li>vpn/vpnc/vpncwidget.cpp <span style="color: grey">(4d5330d)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124279/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>