Re: Review Request 112527: Clean up KEmoticons framework (prior to splitting)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/#review39622 --- Ship it! Ship It! - David Faure On Sept. 9, 2013, 12:05 a.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/ --- (Updated Sept. 9, 2013, 12:05 a.m.) Review request for KDE Frameworks. Description --- Clean up KEmoticons framework (prior to splitting) --Clean up includes --Turn KEmoticonsProvider abstract and reorganize some code --Revise documentation --Use QScopedPointer where useful --Port away from KIO in KEmoticonsProvider --Put const where fit TODO: --Port away from KServiceTypeTrader. Sebas is working on a way to get the plugin list without KServiceTypeTrader. Diffs - staging/kemoticons/autotests/kemoticontest.cpp 0ca1671d26970998c13022daa839e1aae4907220 staging/kemoticons/src/core/kemoticons.h 57912b550c5e941f751d93c084b37764635e11c7 staging/kemoticons/src/core/kemoticons.cpp 317089ed94179aac2b7e448414df930dcfd0c0dd staging/kemoticons/src/core/kemoticonsprovider.h 3f43ca4c8a1fe74ce01a1aac1abdeb84b9da08d2 staging/kemoticons/src/core/kemoticonsprovider.cpp 1fcad79bd49919058bb21b262b57b7a9bb6ac5c9 staging/kemoticons/src/core/kemoticonstheme.h d0e1a899e2f8a89b34e6337848c1aeab7f60ef88 staging/kemoticons/src/core/kemoticonstheme.cpp cd385fde6095b2da930de9ec03a501ae3d1ab0f7 staging/kemoticons/src/providers/adium/adium_emoticons.cpp f465b64e230639f16ca073ed7ab08a79f04afd4a staging/kemoticons/src/providers/kde/kde_emoticons.cpp 7d777b5aa0a869a0009814bb43f84f802444d5d6 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp 65b2c5e979eeccbc98986432962e3ab05f39ca57 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 2535b71263f468dacc830e7cf92fead5e61528e8 staging/kemoticons/tests/main.cpp f46265e97243aa776f901120d832368154c5c2ed Diff: http://git.reviewboard.kde.org/r/112527/diff/ Testing --- It compiles and test passes Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112527: Clean up KEmoticons framework (prior to splitting)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/ --- (Updated Sept. 9, 2013, 8:19 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Description --- Clean up KEmoticons framework (prior to splitting) --Clean up includes --Turn KEmoticonsProvider abstract and reorganize some code --Revise documentation --Use QScopedPointer where useful --Port away from KIO in KEmoticonsProvider --Put const where fit TODO: --Port away from KServiceTypeTrader. Sebas is working on a way to get the plugin list without KServiceTypeTrader. Diffs - staging/kemoticons/autotests/kemoticontest.cpp 0ca1671d26970998c13022daa839e1aae4907220 staging/kemoticons/src/core/kemoticons.h 57912b550c5e941f751d93c084b37764635e11c7 staging/kemoticons/src/core/kemoticons.cpp 317089ed94179aac2b7e448414df930dcfd0c0dd staging/kemoticons/src/core/kemoticonsprovider.h 3f43ca4c8a1fe74ce01a1aac1abdeb84b9da08d2 staging/kemoticons/src/core/kemoticonsprovider.cpp 1fcad79bd49919058bb21b262b57b7a9bb6ac5c9 staging/kemoticons/src/core/kemoticonstheme.h d0e1a899e2f8a89b34e6337848c1aeab7f60ef88 staging/kemoticons/src/core/kemoticonstheme.cpp cd385fde6095b2da930de9ec03a501ae3d1ab0f7 staging/kemoticons/src/providers/adium/adium_emoticons.cpp f465b64e230639f16ca073ed7ab08a79f04afd4a staging/kemoticons/src/providers/kde/kde_emoticons.cpp 7d777b5aa0a869a0009814bb43f84f802444d5d6 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp 65b2c5e979eeccbc98986432962e3ab05f39ca57 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 2535b71263f468dacc830e7cf92fead5e61528e8 staging/kemoticons/tests/main.cpp f46265e97243aa776f901120d832368154c5c2ed Diff: http://git.reviewboard.kde.org/r/112527/diff/ Testing --- It compiles and test passes Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112527: Clean up KEmoticons framework (prior to splitting)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/#review39561 --- one last issue staging/kemoticons/src/core/kemoticonsprovider.h http://git.reviewboard.kde.org/r/112527/#comment29144 absolute path, or relative path? staging/kemoticons/src/core/kemoticonsprovider.h http://git.reviewboard.kde.org/r/112527/#comment29143 absolute path, or relative path? staging/kemoticons/src/core/kemoticonsprovider.cpp http://git.reviewboard.kde.org/r/112527/#comment29145 file.fileName() is the same as emo. Yes, the name of the methods in QFile are a bit confusing. If this code was meant to extract the actual filename only (/tmp/foo.png - foo.png) then you have to use QFileInfo::fileName(). Hence my questions about absolute or relative paths, I don't actually know what emo contains in this method. staging/kemoticons/src/core/kemoticonstheme.h http://git.reviewboard.kde.org/r/112527/#comment29146 same - David Faure On Sept. 8, 2013, 12:58 a.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/ --- (Updated Sept. 8, 2013, 12:58 a.m.) Review request for KDE Frameworks. Description --- Clean up KEmoticons framework (prior to splitting) --Clean up includes --Turn KEmoticonsProvider abstract and reorganize some code --Revise documentation --Use QScopedPointer where useful --Port away from KIO in KEmoticonsProvider --Put const where fit TODO: --Port away from KServiceTypeTrader. Sebas is working on a way to get the plugin list without KServiceTypeTrader. Diffs - staging/kemoticons/src/core/kemoticons.h 57912b550c5e941f751d93c084b37764635e11c7 staging/kemoticons/autotests/kemoticontest.cpp 0ca1671d26970998c13022daa839e1aae4907220 staging/kemoticons/src/core/kemoticons.cpp 317089ed94179aac2b7e448414df930dcfd0c0dd staging/kemoticons/src/core/kemoticonsprovider.h 3f43ca4c8a1fe74ce01a1aac1abdeb84b9da08d2 staging/kemoticons/src/core/kemoticonsprovider.cpp 1fcad79bd49919058bb21b262b57b7a9bb6ac5c9 staging/kemoticons/src/core/kemoticonstheme.h d0e1a899e2f8a89b34e6337848c1aeab7f60ef88 staging/kemoticons/src/core/kemoticonstheme.cpp cd385fde6095b2da930de9ec03a501ae3d1ab0f7 staging/kemoticons/src/providers/adium/adium_emoticons.cpp f465b64e230639f16ca073ed7ab08a79f04afd4a staging/kemoticons/src/providers/kde/kde_emoticons.cpp 7d777b5aa0a869a0009814bb43f84f802444d5d6 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp 65b2c5e979eeccbc98986432962e3ab05f39ca57 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 2535b71263f468dacc830e7cf92fead5e61528e8 staging/kemoticons/tests/main.cpp f46265e97243aa776f901120d832368154c5c2ed Diff: http://git.reviewboard.kde.org/r/112527/diff/ Testing --- It compiles and test passes Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112527: Clean up KEmoticons framework (prior to splitting)
On Sept. 8, 2013, 8:45 a.m., David Faure wrote: staging/kemoticons/src/core/kemoticonsprovider.h, line 85 http://git.reviewboard.kde.org/r/112527/diff/4/?file=188118#file188118line85 absolute path, or relative path? All of them are absolute. Should I specify it in the docs? Or should I modify the code so that it also accepts relative paths and converts them in absolute? On Sept. 8, 2013, 8:45 a.m., David Faure wrote: staging/kemoticons/src/core/kemoticonsprovider.cpp, line 146 http://git.reviewboard.kde.org/r/112527/diff/4/?file=188119#file188119line146 file.fileName() is the same as emo. Yes, the name of the methods in QFile are a bit confusing. If this code was meant to extract the actual filename only (/tmp/foo.png - foo.png) then you have to use QFileInfo::fileName(). Hence my questions about absolute or relative paths, I don't actually know what emo contains in this method. As I said before, emo is an absolute path. - David --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/#review39561 --- On Sept. 8, 2013, 12:58 a.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/ --- (Updated Sept. 8, 2013, 12:58 a.m.) Review request for KDE Frameworks. Description --- Clean up KEmoticons framework (prior to splitting) --Clean up includes --Turn KEmoticonsProvider abstract and reorganize some code --Revise documentation --Use QScopedPointer where useful --Port away from KIO in KEmoticonsProvider --Put const where fit TODO: --Port away from KServiceTypeTrader. Sebas is working on a way to get the plugin list without KServiceTypeTrader. Diffs - staging/kemoticons/src/core/kemoticons.h 57912b550c5e941f751d93c084b37764635e11c7 staging/kemoticons/autotests/kemoticontest.cpp 0ca1671d26970998c13022daa839e1aae4907220 staging/kemoticons/src/core/kemoticons.cpp 317089ed94179aac2b7e448414df930dcfd0c0dd staging/kemoticons/src/core/kemoticonsprovider.h 3f43ca4c8a1fe74ce01a1aac1abdeb84b9da08d2 staging/kemoticons/src/core/kemoticonsprovider.cpp 1fcad79bd49919058bb21b262b57b7a9bb6ac5c9 staging/kemoticons/src/core/kemoticonstheme.h d0e1a899e2f8a89b34e6337848c1aeab7f60ef88 staging/kemoticons/src/core/kemoticonstheme.cpp cd385fde6095b2da930de9ec03a501ae3d1ab0f7 staging/kemoticons/src/providers/adium/adium_emoticons.cpp f465b64e230639f16ca073ed7ab08a79f04afd4a staging/kemoticons/src/providers/kde/kde_emoticons.cpp 7d777b5aa0a869a0009814bb43f84f802444d5d6 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp 65b2c5e979eeccbc98986432962e3ab05f39ca57 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 2535b71263f468dacc830e7cf92fead5e61528e8 staging/kemoticons/tests/main.cpp f46265e97243aa776f901120d832368154c5c2ed Diff: http://git.reviewboard.kde.org/r/112527/diff/ Testing --- It compiles and test passes Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112527: Clean up KEmoticons framework (prior to splitting)
On Sept. 8, 2013, 8:45 a.m., David Faure wrote: staging/kemoticons/src/core/kemoticonsprovider.cpp, line 146 http://git.reviewboard.kde.org/r/112527/diff/4/?file=188119#file188119line146 file.fileName() is the same as emo. Yes, the name of the methods in QFile are a bit confusing. If this code was meant to extract the actual filename only (/tmp/foo.png - foo.png) then you have to use QFileInfo::fileName(). Hence my questions about absolute or relative paths, I don't actually know what emo contains in this method. David Gil Oliva wrote: As I said before, emo is an absolute path. OK then this line of code is broken :) file.fileName() is the same as emo, i.e. an absolute path. Concatenating that to themePath + '/' makes no sense, you'd end up with /usr/path/to/theme/usr/full/path/to/emoticon. On Sept. 8, 2013, 8:45 a.m., David Faure wrote: staging/kemoticons/src/core/kemoticonsprovider.h, line 85 http://git.reviewboard.kde.org/r/112527/diff/4/?file=188118#file188118line85 absolute path, or relative path? David Gil Oliva wrote: All of them are absolute. Should I specify it in the docs? Or should I modify the code so that it also accepts relative paths and converts them in absolute? I guess I got confused by the broken line of code below. Path in KDE usually means absolute, yes. Add full before path, or leave it as is. Definitely don't add support for relative paths if this wasn't there before, it means the base dir is unclear :) - David --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/#review39561 --- On Sept. 8, 2013, 12:58 a.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/ --- (Updated Sept. 8, 2013, 12:58 a.m.) Review request for KDE Frameworks. Description --- Clean up KEmoticons framework (prior to splitting) --Clean up includes --Turn KEmoticonsProvider abstract and reorganize some code --Revise documentation --Use QScopedPointer where useful --Port away from KIO in KEmoticonsProvider --Put const where fit TODO: --Port away from KServiceTypeTrader. Sebas is working on a way to get the plugin list without KServiceTypeTrader. Diffs - staging/kemoticons/src/core/kemoticons.h 57912b550c5e941f751d93c084b37764635e11c7 staging/kemoticons/autotests/kemoticontest.cpp 0ca1671d26970998c13022daa839e1aae4907220 staging/kemoticons/src/core/kemoticons.cpp 317089ed94179aac2b7e448414df930dcfd0c0dd staging/kemoticons/src/core/kemoticonsprovider.h 3f43ca4c8a1fe74ce01a1aac1abdeb84b9da08d2 staging/kemoticons/src/core/kemoticonsprovider.cpp 1fcad79bd49919058bb21b262b57b7a9bb6ac5c9 staging/kemoticons/src/core/kemoticonstheme.h d0e1a899e2f8a89b34e6337848c1aeab7f60ef88 staging/kemoticons/src/core/kemoticonstheme.cpp cd385fde6095b2da930de9ec03a501ae3d1ab0f7 staging/kemoticons/src/providers/adium/adium_emoticons.cpp f465b64e230639f16ca073ed7ab08a79f04afd4a staging/kemoticons/src/providers/kde/kde_emoticons.cpp 7d777b5aa0a869a0009814bb43f84f802444d5d6 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp 65b2c5e979eeccbc98986432962e3ab05f39ca57 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 2535b71263f468dacc830e7cf92fead5e61528e8 staging/kemoticons/tests/main.cpp f46265e97243aa776f901120d832368154c5c2ed Diff: http://git.reviewboard.kde.org/r/112527/diff/ Testing --- It compiles and test passes Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112527: Clean up KEmoticons framework (prior to splitting)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/ --- (Updated Sept. 9, 2013, 12:05 a.m.) Review request for KDE Frameworks. Changes --- Fix issue suggested by David Faure. Description --- Clean up KEmoticons framework (prior to splitting) --Clean up includes --Turn KEmoticonsProvider abstract and reorganize some code --Revise documentation --Use QScopedPointer where useful --Port away from KIO in KEmoticonsProvider --Put const where fit TODO: --Port away from KServiceTypeTrader. Sebas is working on a way to get the plugin list without KServiceTypeTrader. Diffs (updated) - staging/kemoticons/autotests/kemoticontest.cpp 0ca1671d26970998c13022daa839e1aae4907220 staging/kemoticons/src/core/kemoticons.h 57912b550c5e941f751d93c084b37764635e11c7 staging/kemoticons/src/core/kemoticons.cpp 317089ed94179aac2b7e448414df930dcfd0c0dd staging/kemoticons/src/core/kemoticonsprovider.h 3f43ca4c8a1fe74ce01a1aac1abdeb84b9da08d2 staging/kemoticons/src/core/kemoticonsprovider.cpp 1fcad79bd49919058bb21b262b57b7a9bb6ac5c9 staging/kemoticons/src/core/kemoticonstheme.h d0e1a899e2f8a89b34e6337848c1aeab7f60ef88 staging/kemoticons/src/core/kemoticonstheme.cpp cd385fde6095b2da930de9ec03a501ae3d1ab0f7 staging/kemoticons/src/providers/adium/adium_emoticons.cpp f465b64e230639f16ca073ed7ab08a79f04afd4a staging/kemoticons/src/providers/kde/kde_emoticons.cpp 7d777b5aa0a869a0009814bb43f84f802444d5d6 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp 65b2c5e979eeccbc98986432962e3ab05f39ca57 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 2535b71263f468dacc830e7cf92fead5e61528e8 staging/kemoticons/tests/main.cpp f46265e97243aa776f901120d832368154c5c2ed Diff: http://git.reviewboard.kde.org/r/112527/diff/ Testing --- It compiles and test passes Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112527: Clean up KEmoticons framework (prior to splitting)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/ --- (Updated Sept. 8, 2013, 12:58 a.m.) Review request for KDE Frameworks. Changes --- I found some more unused includes in KEmoticonsTheme. Description --- Clean up KEmoticons framework (prior to splitting) --Clean up includes --Turn KEmoticonsProvider abstract and reorganize some code --Revise documentation --Use QScopedPointer where useful --Port away from KIO in KEmoticonsProvider --Put const where fit TODO: --Port away from KServiceTypeTrader. Sebas is working on a way to get the plugin list without KServiceTypeTrader. Diffs (updated) - staging/kemoticons/src/core/kemoticons.h 57912b550c5e941f751d93c084b37764635e11c7 staging/kemoticons/autotests/kemoticontest.cpp 0ca1671d26970998c13022daa839e1aae4907220 staging/kemoticons/src/core/kemoticons.cpp 317089ed94179aac2b7e448414df930dcfd0c0dd staging/kemoticons/src/core/kemoticonsprovider.h 3f43ca4c8a1fe74ce01a1aac1abdeb84b9da08d2 staging/kemoticons/src/core/kemoticonsprovider.cpp 1fcad79bd49919058bb21b262b57b7a9bb6ac5c9 staging/kemoticons/src/core/kemoticonstheme.h d0e1a899e2f8a89b34e6337848c1aeab7f60ef88 staging/kemoticons/src/core/kemoticonstheme.cpp cd385fde6095b2da930de9ec03a501ae3d1ab0f7 staging/kemoticons/src/providers/adium/adium_emoticons.cpp f465b64e230639f16ca073ed7ab08a79f04afd4a staging/kemoticons/src/providers/kde/kde_emoticons.cpp 7d777b5aa0a869a0009814bb43f84f802444d5d6 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp 65b2c5e979eeccbc98986432962e3ab05f39ca57 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 2535b71263f468dacc830e7cf92fead5e61528e8 staging/kemoticons/tests/main.cpp f46265e97243aa776f901120d832368154c5c2ed Diff: http://git.reviewboard.kde.org/r/112527/diff/ Testing --- It compiles and test passes Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112527: Clean up KEmoticons framework (prior to splitting)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/#review39452 --- staging/kemoticons/src/core/kemoticonsprovider.h http://git.reviewboard.kde.org/r/112527/#comment29062 @since 5.0 staging/kemoticons/src/core/kemoticonsprovider.h http://git.reviewboard.kde.org/r/112527/#comment29063 @since 5.0 staging/kemoticons/src/core/kemoticonsprovider.cpp http://git.reviewboard.kde.org/r/112527/#comment29064 This can't possibly work. You are taking a full URL (file:///dir) in string representation and passing that to QFile::copy, which only takes local paths. Drop the QUrl usage. - David Faure On Sept. 5, 2013, 9:02 p.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/ --- (Updated Sept. 5, 2013, 9:02 p.m.) Review request for KDE Frameworks. Description --- Clean up KEmoticons framework (prior to splitting) --Clean up includes --Turn KEmoticonsProvider abstract and reorganize some code --Revise documentation --Use QScopedPointer where useful --Port away from KIO in KEmoticonsProvider --Put const where fit TODO: --Port away from KServiceTypeTrader. Sebas is working on a way to get the plugin list without KServiceTypeTrader. Diffs - staging/kemoticons/autotests/kemoticontest.cpp 0ca1671d26970998c13022daa839e1aae4907220 staging/kemoticons/src/core/kemoticons.h 57912b550c5e941f751d93c084b37764635e11c7 staging/kemoticons/src/core/kemoticons.cpp 317089ed94179aac2b7e448414df930dcfd0c0dd staging/kemoticons/src/core/kemoticonsprovider.h 3f43ca4c8a1fe74ce01a1aac1abdeb84b9da08d2 staging/kemoticons/src/core/kemoticonsprovider.cpp 1fcad79bd49919058bb21b262b57b7a9bb6ac5c9 staging/kemoticons/src/core/kemoticonstheme.h d0e1a899e2f8a89b34e6337848c1aeab7f60ef88 staging/kemoticons/src/providers/adium/adium_emoticons.cpp f465b64e230639f16ca073ed7ab08a79f04afd4a staging/kemoticons/src/providers/kde/kde_emoticons.cpp 7d777b5aa0a869a0009814bb43f84f802444d5d6 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp 65b2c5e979eeccbc98986432962e3ab05f39ca57 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 2535b71263f468dacc830e7cf92fead5e61528e8 staging/kemoticons/tests/main.cpp f46265e97243aa776f901120d832368154c5c2ed Diff: http://git.reviewboard.kde.org/r/112527/diff/ Testing --- It compiles and test passes Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112527: Clean up KEmoticons framework (prior to splitting)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/#review39394 --- staging/kemoticons/src/providers/adium/adium_emoticons.cpp http://git.reviewboard.kde.org/r/112527/#comment29027 Shouldn't you return false here? staging/kemoticons/src/providers/kde/kde_emoticons.cpp http://git.reviewboard.kde.org/r/112527/#comment29028 return false as well here staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp http://git.reviewboard.kde.org/r/112527/#comment29029 return false staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp http://git.reviewboard.kde.org/r/112527/#comment29030 return false staging/kemoticons/tests/main.cpp http://git.reviewboard.kde.org/r/112527/#comment29031 while you are at it, can you clean the trailing whitespaces? - Aurélien Gâteau On Sept. 5, 2013, 12:47 a.m., David Gil Oliva wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/ --- (Updated Sept. 5, 2013, 12:47 a.m.) Review request for KDE Frameworks. Description --- Clean up KEmoticons framework (prior to splitting) --Clean up includes --Turn KEmoticonsProvider abstract and reorganize some code --Revise documentation --Use QScopedPointer where useful --Port away from KIO in KEmoticonsProvider --Put const where fit TODO: --Port away from KServiceTypeTrader. Sebas is working on a way to get the plugin list without KServiceTypeTrader. Diffs - staging/kemoticons/autotests/kemoticontest.cpp 0ca1671d26970998c13022daa839e1aae4907220 staging/kemoticons/src/core/kemoticons.h 57912b550c5e941f751d93c084b37764635e11c7 staging/kemoticons/src/core/kemoticons.cpp 317089ed94179aac2b7e448414df930dcfd0c0dd staging/kemoticons/src/core/kemoticonsprovider.h 3f43ca4c8a1fe74ce01a1aac1abdeb84b9da08d2 staging/kemoticons/src/core/kemoticonsprovider.cpp 1fcad79bd49919058bb21b262b57b7a9bb6ac5c9 staging/kemoticons/src/core/kemoticonstheme.h d0e1a899e2f8a89b34e6337848c1aeab7f60ef88 staging/kemoticons/src/providers/adium/adium_emoticons.cpp f465b64e230639f16ca073ed7ab08a79f04afd4a staging/kemoticons/src/providers/kde/kde_emoticons.cpp 7d777b5aa0a869a0009814bb43f84f802444d5d6 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp 65b2c5e979eeccbc98986432962e3ab05f39ca57 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 2535b71263f468dacc830e7cf92fead5e61528e8 staging/kemoticons/tests/main.cpp f46265e97243aa776f901120d832368154c5c2ed Diff: http://git.reviewboard.kde.org/r/112527/diff/ Testing --- It compiles and test passes Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112527: Clean up KEmoticons framework (prior to splitting)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/ --- (Updated Sept. 5, 2013, 9:02 p.m.) Review request for KDE Frameworks. Changes --- Fix issues suggested by Aurélien Gateau Description --- Clean up KEmoticons framework (prior to splitting) --Clean up includes --Turn KEmoticonsProvider abstract and reorganize some code --Revise documentation --Use QScopedPointer where useful --Port away from KIO in KEmoticonsProvider --Put const where fit TODO: --Port away from KServiceTypeTrader. Sebas is working on a way to get the plugin list without KServiceTypeTrader. Diffs (updated) - staging/kemoticons/autotests/kemoticontest.cpp 0ca1671d26970998c13022daa839e1aae4907220 staging/kemoticons/src/core/kemoticons.h 57912b550c5e941f751d93c084b37764635e11c7 staging/kemoticons/src/core/kemoticons.cpp 317089ed94179aac2b7e448414df930dcfd0c0dd staging/kemoticons/src/core/kemoticonsprovider.h 3f43ca4c8a1fe74ce01a1aac1abdeb84b9da08d2 staging/kemoticons/src/core/kemoticonsprovider.cpp 1fcad79bd49919058bb21b262b57b7a9bb6ac5c9 staging/kemoticons/src/core/kemoticonstheme.h d0e1a899e2f8a89b34e6337848c1aeab7f60ef88 staging/kemoticons/src/providers/adium/adium_emoticons.cpp f465b64e230639f16ca073ed7ab08a79f04afd4a staging/kemoticons/src/providers/kde/kde_emoticons.cpp 7d777b5aa0a869a0009814bb43f84f802444d5d6 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp 65b2c5e979eeccbc98986432962e3ab05f39ca57 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 2535b71263f468dacc830e7cf92fead5e61528e8 staging/kemoticons/tests/main.cpp f46265e97243aa776f901120d832368154c5c2ed Diff: http://git.reviewboard.kde.org/r/112527/diff/ Testing --- It compiles and test passes Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel