[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-frameworks-devel
Subject: Re: Review Request 117777: Make the KSelectionProxyModel unit test pass
From: "Stephen Kelly" <steveire () gmail ! com>
Date: 2014-04-26 10:22:01
Message-ID: 20140426102201.23578.12635 () probe ! kde ! org
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On April 26, 2014, 8:51 a.m., Stephen Kelly wrote:
> > Would it be a good idea to start by reverting \
> > https://git.reviewboard.kde.org/r/116096/ ?
>
> Alex Merry wrote:
> So we have no unit tests? I don't really see how that would help (except in the \
> strict sense of no unit tests => no failing unit tests).
I meant considering the problem again as it was before that patch - namely doing a \
proper merge of the two versions. Is that what this patch does? Is it a full merge?
- Stephen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117777/#review56594
-----------------------------------------------------------
On April 25, 2014, 7:18 p.m., Alex Merry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117777/
> -----------------------------------------------------------
>
> (Updated April 25, 2014, 7:18 p.m.)
>
>
> Review request for KDE Frameworks and Stephen Kelly.
>
>
> Repository: kitemmodels
>
>
> Description
> -------
>
> Make the KSelectionProxyModel unit test pass
>
> It looks like Qt4 emitted layoutChanged in circumstances where Qt5 emits
> rowsMoved. This could be used to optimize things, but for now we just
> act the same way in either case (rebuilding the mapping).
>
> Add a pedantic mode to modeltest.
>
> Cherry-pick of ffd732966d1f2c632d811e8fc378a95aa1456703
>
> This was lost when I updated ModelTest to Qt5.
>
> Re-apply Stephen Kelly's modeltest fixes
>
> These were lost when I replaced ModelTest with the version from Qt 5.
>
>
> Diffs
> -----
>
> autotests/CMakeLists.txt 4281fec2de1124162af623cf999db73eff705684
> autotests/proxymodeltestsuite/modeltest.h 4b80850d51ca068fb8ffb0a704a9f938b4dacfa9
> autotests/proxymodeltestsuite/modeltest.cpp \
> 0928a3a4d78d10fcae4a5671e91e836e9fde429b src/kselectionproxymodel.cpp \
> ea97f7a197f9181745f2979c2f7b2e5e1f0015f3
> Diff: https://git.reviewboard.kde.org/r/117777/diff/
>
>
> Testing
> -------
>
> Unit tests now pass.
>
>
> Thanks,
>
> Alex Merry
>
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/117777/">https://git.reviewboard.kde.org/r/117777/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On April 26th, 2014, 8:51 a.m. UTC, <b>Stephen \
Kelly</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Would it be a good idea to start by reverting \
https://git.reviewboard.kde.org/r/116096/ ?</pre> </blockquote>
<p>On April 26th, 2014, 9:09 a.m. UTC, <b>Alex Merry</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">So we have no unit \
tests? I don't really see how that would help (except in the strict sense of no \
unit tests => no failing unit tests).</pre> </blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I meant considering the \
problem again as it was before that patch - namely doing a proper merge of the two \
versions. Is that what this patch does? Is it a full merge?</pre> <br />
<p>- Stephen</p>
<br />
<p>On April 25th, 2014, 7:18 p.m. UTC, Alex Merry wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for KDE Frameworks and Stephen Kelly.</div>
<div>By Alex Merry.</div>
<p style="color: grey;"><i>Updated April 25, 2014, 7:18 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kitemmodels
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Make the KSelectionProxyModel unit test pass
It looks like Qt4 emitted layoutChanged in circumstances where Qt5 emits
rowsMoved. This could be used to optimize things, but for now we just
act the same way in either case (rebuilding the mapping).
Add a pedantic mode to modeltest.
Cherry-pick of ffd732966d1f2c632d811e8fc378a95aa1456703
This was lost when I updated ModelTest to Qt5.
Re-apply Stephen Kelly's modeltest fixes
These were lost when I replaced ModelTest with the version from Qt 5.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Unit tests now pass.</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>autotests/CMakeLists.txt <span style="color: \
grey">(4281fec2de1124162af623cf999db73eff705684)</span></li>
<li>autotests/proxymodeltestsuite/modeltest.h <span style="color: \
grey">(4b80850d51ca068fb8ffb0a704a9f938b4dacfa9)</span></li>
<li>autotests/proxymodeltestsuite/modeltest.cpp <span style="color: \
grey">(0928a3a4d78d10fcae4a5671e91e836e9fde429b)</span></li>
<li>src/kselectionproxymodel.cpp <span style="color: \
grey">(ea97f7a197f9181745f2979c2f7b2e5e1f0015f3)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/117777/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic