<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 />
<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;">Changes</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;">Update:
Changed PersonData to be a sharedpointer.
In the old code PersonsDetailView deleted the passed PersonData that was last given to it, when you set a new person data.
If we were to use this in real life we would have crashes everywhere.
For objects shared in multiple places, created and deleted during the runtime of the program sharedptrs are the safest solution.
Due to the above changes, this will still work in QML which has it's own shared pointer solution.</pre>
</td>
</tr>
</table>
<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> (updated)</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>