<table><tr><td style="">kossebau 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/D8588" 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/D8588#163592" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;" rel="noreferrer">D8588#163592</a>, <a href="https://phabricator.kde.org/p/mwolff/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;" rel="noreferrer">@mwolff</a> wrote:</div>
<div style="margin: 0;
          padding: 0;
          border: 0;
          color: rgb(107, 116, 140);"><p>Big change is big. Why did you bother doing it? Do you have performance measurements indicating that this is a bottleneck? If so, do you want to save memory or do you want to make things faster?</p></div>
</blockquote>

<p>Motivation was: had seen quite some vcs kdevplatform code & API usage recently, and when noticing in code those classes do deep copies every time, it seemed they should become shared data classes rather (especially as that change was just a few lines to do and done in a few minutes, other than the hours spent on experimenting with the unit tests).</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>Would it maybe be enough to add move semantics?</p></blockquote>

<p>No, as the lifetime is: each item generated as part of group of items representing VCS data read in, then being kept around for read-only access in containers. Until there move semantics would work I guess,. But the items in the containers are then also partially accessed via a querying API that returns the items by value. Where then shared data seems a better approach over deep copies.</p>

<p>But in only-do-changes-after-measurement class I deserve a bad grade, no single measurement done. The new implementation just "felt" better by principles.</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>I'm not in principle opposed to this patch, considering the work is done. Though for the future, please try to trim down the verbosity of the tests. They are really much too big. I.e. testing the permutations of changing A and then changing B is pretty pointless imo.</p></blockquote>

<p>Okay, could drop the B ones, more a left-over from my experiments how to do a full coverage, agreed it does not add much knowing the implementation. Will update later tonight.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R32 KDevelop</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D8588" rel="noreferrer">https://phabricator.kde.org/D8588</a></div></div><br /><div><strong>To: </strong>kossebau, KDevelop<br /><strong>Cc: </strong>mwolff, brauch, kdevelop-devel<br /></div>