<table><tr><td style="">dfaure added a comment.
</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/D7829" rel="noreferrer">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
          color: #6B748C;
          font-style: italic;
          margin: 4px 0 12px 0;
          padding: 8px 12px;
          background-color: #F8F9FC;">
<div style="font-style: normal;
          padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D7829#146061" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;" rel="noreferrer">D7829#146061</a>, <a href="https://phabricator.kde.org/p/graesslin/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;" rel="noreferrer">@graesslin</a> wrote:</div>
<div style="margin: 0;
          padding: 0;
          border: 0;
          color: rgb(107, 116, 140);"><p>How do we know that this doesn't break other code? This code is for example used in KWin's Alt+Tab handling which is it's own fair beast. Any changes here might break the code as it might be bug-to-bug compatible.</p></div>
</blockquote>

<p>I tested that Alt+Tab / Alt+Shift+Tab still works, and put that into the unittest. Anything else that worries you?</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>We had the changes one month in master and nobody noticed the severe brokeness, how are we going to ensure that further patches to this extremely fragile code will not introduce further regressions?</p></blockquote>

<p>By unittesting the main key handling method, not just the additional conversion methods, and by committing the changes at the very beginning of a release cycle to maximize testing.<br />
With the additional testing I am more confident now about this patch.<br />
I think it should go in - but indeed certainly not for 5.39, rather just after 5.39.</p>

<p>The right solution when code is fragile and we fear changing it, is to make it less fragile, by removing duplication (done), adding unittests (done) and doing more user-testing (to be done again...).<br />
Leaving such code untouched because we fear touching it, is not the right solution.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R278 KWindowSystem</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D7829" rel="noreferrer">https://phabricator.kde.org/D7829</a></div></div><br /><div><strong>To: </strong>dfaure, graesslin, jriddell, martinkostolny, broulik<br /><strong>Cc: </strong>Frameworks<br /></div>