> On 2010-11-09 00:33:12, Raphael Kubo da Costa wrote: > > trunk/KDE/kdelibs/kdeui/CMakeLists.txt, line 43 > > <http://svn.reviewboard.kde.org/r/5794/diff/4/?file=40868#file40868line43> > > > > Since attica is a required dependency, I'd rather use a more specific > > name here, such as KDEUI_NO_ATTICA itself.
In that case, in the code I'd have #ifndef KDEUI_NO_ATTICA rather than #ifdef HAVE_ATTICA, so I figured I'd rather avoid a double negation. Also a quick ack grep in KDElibs tells me that HAVE_STUFF seems to be a common naming policy for CMake defines used for conditional compilation of code. - Teo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5794/#review8574 ----------------------------------------------------------- On 2010-11-09 09:22:50, Teo Mrnjavac wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://svn.reviewboard.kde.org/r/5794/ > ----------------------------------------------------------- > > (Updated 2010-11-09 09:22:50) > > > Review request for kdelibs. > > > Summary > ------- > > The present review request contains the implementation of a Social About > Dialog feature for KDE apps, and is a rewrite of Amarok's Social About Dialog > (See > http://teom.wordpress.com/2009/08/23/social-desktop-integration-in-kaboutdialog/ > ). For those who don't have time to read through that link, the Social About > Dialog is an evolution of our old KAboutDialog which adds optional Open > Collaboration Services features to complete the contributors' data. > The only changes that concern the API are in kaboutdata.{h,cpp}, and I've > tried to keep it binary compatible to the best of my abilities by following > the instructions on our Techbase, I hope I did it right. > The only new dependency is the Attica library, except if we are building with > KDE_PLATFORM_FEATURE_BINARY_COMPATIBLE_FEATURE_REDUCTION, in that case all > social features are #ifdef'ed out and the dialog behaves very much like the > old KAboutDialog. > The dialog itself uses model/view/delegate, not very different from > KNewStuff3, and most of the action happens in the model (jobs to get data > from Attica/OCS) and in the delegate. > As we're getting dangerously close to the string freeze, I've decided to put > this up for review even though there are still a couple TODOs in the code. > These are: > * The icons are placeholders, we are going to need some real icons for links > and social networks. We already have some of those for Amarok's about dialog > but this needs to be cleanly solved in a way that would not make us liable > for IP infringement in any way. IANAL here so a discussion as to how to > proceed is welcome. > * In the optional translators tab there's a short but not very short text > about the translation team. I've added it as a label but it tends to take a > bit more room than I'd like, so I'm requesting for comments on what to do > with it. Also, I don't know how translators' names are added. If translators > can't add their OCS usernames anyway then I'll easily revert to the old about > view for them and the problem of the tall translation team text is > automatically solved. > * Since kdeui does not depend on KIO, I couldn't use KIO::get and had to use > QNAM instead. For the same reason I had to use QDesktopServices instead of > KRun::runUrl() to launch URLs. As it is now, stuff works, but to make it > nicer the plan could be to add an Attica::PixmapFromUrlJob (which I've been > told would satisfy other unrelated needs too) to Attica to remove the single > instance where QNAM is used instead of KIO. > > Except maybe for the translators tab issues, none of the TODOs require string > changes. > > To test the feature you only need to compile KDElibs with the patch and run > any KDE app which has an about dialog, however you won't be able to try the > social features unless you also add OCS usernames (by default the provider is > openDesktop.org) to one or more contributors in the KAboutData of a KDE app > of your choice. > For example: aboutData.addAuthor( ki18n("Random Dude") , ki18n("Random > stuff"), "randomd...@kde.org", "http://example.com";, > "random-dude's-oD.o-username"); > > > Diffs > ----- > > trunk/KDE/kdelibs/kdecore/kernel/kaboutdata.h 1194020 > trunk/KDE/kdelibs/kdecore/kernel/kaboutdata.cpp 1194020 > trunk/KDE/kdelibs/kdeui/CMakeLists.txt 1194020 > trunk/KDE/kdelibs/kdeui/dialogs/kaboutapplicationconfigattica_p.h.cmake > PRE-CREATION > trunk/KDE/kdelibs/kdeui/dialogs/kaboutapplicationdialog.h 1194020 > trunk/KDE/kdelibs/kdeui/dialogs/kaboutapplicationdialog.cpp 1194020 > trunk/KDE/kdelibs/kdeui/dialogs/kaboutapplicationpersonlistdelegate_p.h > PRE-CREATION > trunk/KDE/kdelibs/kdeui/dialogs/kaboutapplicationpersonlistdelegate_p.cpp > PRE-CREATION > trunk/KDE/kdelibs/kdeui/dialogs/kaboutapplicationpersonlistview_p.h > PRE-CREATION > trunk/KDE/kdelibs/kdeui/dialogs/kaboutapplicationpersonlistview_p.cpp > PRE-CREATION > trunk/KDE/kdelibs/kdeui/dialogs/kaboutapplicationpersonmodel_p.h > PRE-CREATION > trunk/KDE/kdelibs/kdeui/dialogs/kaboutapplicationpersonmodel_p.cpp > PRE-CREATION > trunk/KDE/kdelibs/kdeui/dialogs/thumb_frame.png UNKNOWN > > Diff: http://svn.reviewboard.kde.org/r/5794/diff > > > Testing > ------- > > Testing during development, both with Attica enabled and disabled. > > > Screenshots > ----------- > > The social about dialog as it appears when OCS usernames for openDesktop.org > are provided in addAuthor() > http://svn.reviewboard.kde.org/r/5794/s/552/ > > > Thanks, > > Teo > >
