dfaure added a comment.
In https://phabricator.kde.org/D7829#146061, @graesslin wrote: > 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. I tested that Alt+Tab / Alt+Shift+Tab still works, and put that into the unittest. Anything else that worries you? > 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? 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. With the additional testing I am more confident now about this patch. I think it should go in - but indeed certainly not for 5.39, rather just after 5.39. 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...). Leaving such code untouched because we fear touching it, is not the right solution. REPOSITORY R278 KWindowSystem REVISION DETAIL https://phabricator.kde.org/D7829 To: dfaure, graesslin, jriddell, martinkostolny, broulik Cc: #frameworks
