<table><tr><td style="">joselema 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/D11508">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/D11508#231341" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;">D11508#231341</a>, <a href="https://phabricator.kde.org/p/mlaurent/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@mlaurent</a> wrote:</div>
<div style="margin: 0;
          padding: 0;
          border: 0;
          color: rgb(107, 116, 140);"><p>No as we use this include in MessageList/Widget</p>

<p>So we need to install it.</p></div>
</blockquote>

<p>Well that seems to me exactly the opposite of what you said above; I sent the second version of the patch because you said above:</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>For me the second patch seems more logical.</p></blockquote>



<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>Perhaps you can test it.<br />
Indeed we don't need to install it.</p></blockquote>

<p>So it's my understanding that you said that "we don't need to install widgetbase.h".</p>

<p>However, I tested the second patch against the stable branch, and I just realized it wouldn't build against master because of:<br />
<a href="https://cgit.kde.org/messagelib.git/commit/?id=e068f81e9a6176309301eed7765aa54b2ef120dc" class="remarkup-link" target="_blank" rel="noreferrer">https://cgit.kde.org/messagelib.git/commit/?id=e068f81e9a6176309301eed7765aa54b2ef120dc</a></p>

<p>So using #include "core/widgetbase.h" instead of the camelcase <MessageList/Widget> would fix the compilation of the second patch against master.</p>

<p>So, I'm sending a third version of the patch in case your first quoted comment is right and the second one is wrong (i.e. the installation of widgetbase.h is not actually neccesary). If it's the opossite we can just close the review.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R94 PIM: Message Library</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D11508">https://phabricator.kde.org/D11508</a></div></div><br /><div><strong>To: </strong>joselema, KDE PIM, KDE PIM: KMail, mlaurent<br /><strong>Cc: </strong>mlaurent, joselema, rikmills, dvasin, hrouis, ach, winterz, vkrause, knauss, dvratil<br /></div>