<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="http://git.reviewboard.kde.org/r/111308/">http://git.reviewboard.kde.org/r/111308/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This review has been submitted with commit b3a7d1e70aa43074ffb326315e492d6a73af0b62 by David Edmundson to branch master.</pre>
<br />
<p>- Commit</p>
<br />
<p>On June 30th, 2013, 10:33 p.m. UTC, David Edmundson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://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 Telepathy, Aleix Pol Gonzalez and Vishesh Handa.</div>
<div>By David Edmundson.</div>
<p style="color: grey;"><i>Updated June 30, 2013, 10:33 p.m.</i></p>
<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;">Disallow changing URI of PersonData after creation
Currently setUri and setContactId are public which would cause a lot of bugs if changed.
Instead replace with two clear static methods; loadFromUri and loadFromContactId.
In practice they were never used more than once.
A declarative wrapper using QDeclarativeParserStatus provides the same functionality
in QML. A wrapper is needed anyway as it means we can now put more complex data types in PersonData.
bugs that used to exist which are no longer relevant:
- every single plugin for PersonsViewer does not respond to uriChanged
- personsactionsmodel did not respond to uriChanged()
- contactId would return the last user set with setContactId not the current contacts ID (which is misleading/wrong anyway, as a person can have multiple IDs)
- isPerson property was not updated when changed
- PeronDetailsView kept an invalid empty PersonData on construction with a lifespan of the parent.. for no reason.
Given we want people to write plugins, we need to make it easier.
PersonData code is also simpler as it doesn't have to be careful about wiping old resources in setUri.
Downside is you can't have a PersonData on the stack. Bit of a effort for the unit tests.. but not in the real world I don't think.
I believe someone (vishesh?) also suggested this at the Pineda sprint. </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>src/abstractpersonplugin.h <span style="color: grey">(60508b693fd6406b77f4622bff4c4419fb13c50e)</span></li>
<li>src/abstractpersonplugin.cpp <span style="color: grey">(3a41f3bcff668994de70d77c77fd5a4a061dadde)</span></li>
<li>src/autotests/tests/persondatatests.cpp <span style="color: grey">(623fb5f752cca6d98aa261a72a34b740d74b92e8)</span></li>
<li>src/declarative/CMakeLists.txt <span style="color: grey">(9b33b85429c65ce832f37edecebf2ecdf467c476)</span></li>
<li>src/declarative/declarativepersondata.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/declarative/declarativepersondata.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/declarative/peopleqmlplugin.cpp <span style="color: grey">(c991064e011f5115683adb90955c0e3a5dbd156c)</span></li>
<li>src/examples/personwidget.cpp <span style="color: grey">(bc2eaf2805891a201fbbf8cb20420764652e34c7)</span></li>
<li>src/personactionsmodel.cpp <span style="color: grey">(458006213442bcd24bb08216594aed1deb7fb171)</span></li>
<li>src/persondata.h <span style="color: grey">(177f2451c5c432d3cb6add454716bd292540bb7f)</span></li>
<li>src/persondata.cpp <span style="color: grey">(2341bd4f4215bcc2a671f75a118c488e870c98ac)</span></li>
<li>src/personitem.h <span style="color: grey">(91fcd3cc6558625622d17b02805cc4f04382c51d)</span></li>
<li>src/personitem.cpp <span style="color: grey">(6086e41afbd028d830c6a3771dad9fd69d3627cc)</span></li>
<li>src/personpluginmanager.h <span style="color: grey">(38f13816485e69a4a2acab45b73c978d6448be1a)</span></li>
<li>src/personpluginmanager.cpp <span style="color: grey">(487f1bbde1cf122b4c475d8a33bc7514bc9bcc43)</span></li>
<li>src/personsmodel.h <span style="color: grey">(4b6a829d5c3b62e7f057fd15c821460b1e8c5df3)</span></li>
<li>src/personsmodel.cpp <span style="color: grey">(813d0afd6add7d8c33a9d08729ce9c5acadc421f)</span></li>
<li>src/plugins/core/emailplugin.h <span style="color: grey">(ebffe6451b0a8cd74d934227e1f771643acab402)</span></li>
<li>src/plugins/core/emailplugin.cpp <span style="color: grey">(83babd4aa71627d6172794a6878a15b0738ad15e)</span></li>
<li>src/resourcewatcherservice.h <span style="color: grey">(94210ef7ac754eebd324c3d55c391378e1dc07d9)</span></li>
<li>src/resourcewatcherservice.cpp <span style="color: grey">(1342c99a597a3203c17aeb187ec2740af1e0ed00)</span></li>
<li>src/widgets/persondetailsview.cpp <span style="color: grey">(f655eb2ec0a16f00239b679263354632a6183fee)</span></li>
<li>src/widgets/plugins/mergecontactswidget.cpp <span style="color: grey">(51e11dc8d2457297336616e6ee211493b0f5fa08)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111308/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>