<table><tr><td style="">alexeymin 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/D6096">View Revision</a></tr></table><br /><div><div><p>While I'm here and still remember some things that I don't like here - naming:</p>
<p>There is a class <tt style="background: #ebebeb; font-size: 13px;">EventsPlugin</tt> and files <tt style="background: #ebebeb; font-size: 13px;">eventsplugin.{h,cpp}</tt>, class <tt style="background: #ebebeb; font-size: 13px;">EventsManager</tt> and files <tt style="background: #ebebeb; font-size: 13px;">eventsmanager.{h,cpp}</tt> and this is fine.<br />
But there is also class <tt style="background: #ebebeb; font-size: 13px;">EventHandler</tt>, but files are called <tt style="background: #ebebeb; font-size: 13px;">events.{h,cpp}</tt>, it makes it a little bit harder to quickly read the code; maybe it makes sense to rename file also to <tt style="background: #ebebeb; font-size: 13px;">eventhandler.h</tt> ?<br />
Also class <tt style="background: #ebebeb; font-size: 13px;">FakeInputEventHandler</tt> in file <tt style="background: #ebebeb; font-size: 13px;">fakeinputevents.h</tt>; it may be also better to name the file <tt style="background: #ebebeb; font-size: 13px;">fakeinputeventhandler.h</tt>...</p>
<p>And the most important suggestion I have: can this work be split in 2 parts:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">first part, split event handling into a separate type of plugin, and add a new event handler plugins, so we can test it on X11 first (even without wayland) and be sure it still works? (Hopefully this part can be done fast enough)</li>
<li class="remarkup-list-item">second part, add a new shiny GBM framebuffer plugin, and new dependencies on libdrm, libgbm, epoxy...</li>
</ul>
<p>Will be a bit easier to review and test, step by step.<br />
What do you think?</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R437 Desktop Sharing</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D6096">https://phabricator.kde.org/D6096</a></div></div><br /><div><strong>To: </strong>Kanedias, davidedmundson, graesslin<br /><strong>Cc: </strong>alexeymin, plasma-devel, ragreen, schernikov, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein<br /></div>