Re: Review Request 112527: Clean up KEmoticons framework (prior to splitting)

2013-09-09 Thread David Faure

---
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)

2013-09-09 Thread Commit Hook

---
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)

2013-09-08 Thread David Faure

---
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)

2013-09-08 Thread David Gil Oliva


 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)

2013-09-08 Thread David Faure


 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)

2013-09-08 Thread David Gil Oliva

---
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)

2013-09-07 Thread David Gil Oliva

---
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)

2013-09-06 Thread David Faure

---
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)

2013-09-05 Thread Aurélien Gâteau

---
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)

2013-09-05 Thread David Gil Oliva

---
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