----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111308/#review35349 -----------------------------------------------------------
Ship it! Watch out for personitem.cpp, looks like you have my patch applied. src/persondata.cpp <http://git.reviewboard.kde.org/r/111308/#comment25899> & uri -> &uri src/persondata.cpp <http://git.reviewboard.kde.org/r/111308/#comment25900> & contacItd -> &contactId src/persondata.cpp <http://git.reviewboard.kde.org/r/111308/#comment25901> I know it's the original code, but I think this could also use "if (d->uri == uri) { return; } src/widgets/plugins/mergecontactswidget.cpp <http://git.reviewboard.kde.org/r/111308/#comment25902> This shouldn't be part of this patch (it's fixed in my branch) - Martin Klapetek On June 30, 2013, 10:33 p.m., David Edmundson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111308/ > ----------------------------------------------------------- > > (Updated June 30, 2013, 10:33 p.m.) > > > Review request for Telepathy, Aleix Pol Gonzalez and Vishesh Handa. > > > Description > ------- > > 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. > > > Diffs > ----- > > src/abstractpersonplugin.h 60508b693fd6406b77f4622bff4c4419fb13c50e > src/abstractpersonplugin.cpp 3a41f3bcff668994de70d77c77fd5a4a061dadde > src/autotests/tests/persondatatests.cpp > 623fb5f752cca6d98aa261a72a34b740d74b92e8 > src/declarative/CMakeLists.txt 9b33b85429c65ce832f37edecebf2ecdf467c476 > src/declarative/declarativepersondata.h PRE-CREATION > src/declarative/declarativepersondata.cpp PRE-CREATION > src/declarative/peopleqmlplugin.cpp > c991064e011f5115683adb90955c0e3a5dbd156c > src/examples/personwidget.cpp bc2eaf2805891a201fbbf8cb20420764652e34c7 > src/personactionsmodel.cpp 458006213442bcd24bb08216594aed1deb7fb171 > src/persondata.h 177f2451c5c432d3cb6add454716bd292540bb7f > src/persondata.cpp 2341bd4f4215bcc2a671f75a118c488e870c98ac > src/personitem.h 91fcd3cc6558625622d17b02805cc4f04382c51d > src/personitem.cpp 6086e41afbd028d830c6a3771dad9fd69d3627cc > src/personpluginmanager.h 38f13816485e69a4a2acab45b73c978d6448be1a > src/personpluginmanager.cpp 487f1bbde1cf122b4c475d8a33bc7514bc9bcc43 > src/personsmodel.h 4b6a829d5c3b62e7f057fd15c821460b1e8c5df3 > src/personsmodel.cpp 813d0afd6add7d8c33a9d08729ce9c5acadc421f > src/plugins/core/emailplugin.h ebffe6451b0a8cd74d934227e1f771643acab402 > src/plugins/core/emailplugin.cpp 83babd4aa71627d6172794a6878a15b0738ad15e > src/resourcewatcherservice.h 94210ef7ac754eebd324c3d55c391378e1dc07d9 > src/resourcewatcherservice.cpp 1342c99a597a3203c17aeb187ec2740af1e0ed00 > src/widgets/persondetailsview.cpp f655eb2ec0a16f00239b679263354632a6183fee > src/widgets/plugins/mergecontactswidget.cpp > 51e11dc8d2457297336616e6ee211493b0f5fa08 > > Diff: http://git.reviewboard.kde.org/r/111308/diff/ > > > Testing > ------- > > > Thanks, > > David Edmundson > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
