<table><tr><td style="">jgrulich 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/D20930">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/D20930#inline-118661">View Inline</a><span style="color: #4b4d51; font-weight: bold;">andersonbruce</span> wrote in <span style="color: #4b4d51; font-weight: bold;">connectionicon.cpp:403</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">There is still a discrepancy between the enum defined by NetworkManager and the one defined by NMQT. NetworkManager is returning a value of 29 but NetworkManager::Device::WireGuard has a value of 30.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">This is correct, we are unfortunately +1 because back then there was a device which we add support for and later during the NM development cycle it was renamed and I couldn't just simply rename or remove an enum, because it would break ABI. You can see in NMQT that there is code converting NM device type to NMQT device type so we this coverd.</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/D20930#inline-118663">View Inline</a><span style="color: #4b4d51; font-weight: bold;">andersonbruce</span> wrote in <span style="color: #4b4d51; font-weight: bold;">wireguardpeerwidget.cpp:52</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">SimpleIpV4AddressValidator and SimpleIpV6AddressValidator were intentionally done this way because they were existing functions that I didn't want to break when I added new (defaulted) parameters for use in with WireGuard functionality.</p>
<p style="padding: 0; margin: 8px;">Since changing these two will involve changes to non-WireGuard code and it makes sense to do them all at the same time, can we write this up as a separate Bug?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Ok, we can do that in a separate review.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R116 Plasma Network Management Applet</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D20930">https://phabricator.kde.org/D20930</a></div></div><br /><div><strong>To: </strong>andersonbruce, jgrulich, VDG, ngraham<br /><strong>Cc: </strong>ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart<br /></div>