Re: [Telepathy] Work on telepathy-qt service
Hello George, thanks for your comments! I did everything you suggested. 1) Split the classes into single commits, squashed all other commits. 2) Cleanup the coding style, additionally run the astyle command from kdelibs style page. 3) Added copyright and license 4) Added pretty headers for every class Took me quite some while, though. You can find the update branch at https://github.com/mgehre/telepathy-qt/tree/master The cmake change for QT5 is at https://github.com/mgehre/telepathy-qt/tree/qt5 if you want to run it under QT5 final. Best wishes, Matthias 2013/3/25 George Kiagiadakis > On Mon, Mar 11, 2013 at 6:34 AM, Matthias Gehre wrote: > > Hi, > > > > I have been working on a connection manager for whatsapp [1] using > > telepathy-qt. As the telepathy-qt service part is lacking, I implemented > [2] > > the interfaces I needed (see appendix). My changes mainly add code and > only > > touch code in the service part (which is not built by default), so I hope > > this can be merged. I would like your opinions on what how to improve the > > code before merging. > > > > The branch also includes three generic commits: One which replaces > > QString::toAscii with QString::toLatin1, because the former is no longer > > available in QT 5. On which does the same for QWeakPointer -> QPointer. > > > > And a commit that makes cmake find QT5. Do not merge that commit, it will > > break QT4. Some better way has to be found there. > > > > Best wishes, > > Matthias > > > > Implemented interfaces: > > BaseChannel, > > BaseChannelTextType, > > BaseChannelMessagesInterface, > > BaseChannelServerAuthenticationType, > > BaseChannelCaptchaAuthenticationInterface, > > BaseConnectionRequestsInterface > > BaseConnectionContactsInterface > > BaseConnectionSimplePresenceInterface > > BaseConnectionContactListInterface > > BaseConnectionAddressingInterface > > > > > > [1] https://github.com/mgehre/telepathy-whosthere > > [2] https://github.com/mgehre/telepathy-qt > > > > Hello, > > Sorry for the delayed reply. It is great to see somebody is working on > this feature of tp-qt and that there is at last a connection manager > implemented with Qt :) > > I had a quick look at your branch and it looks like you are doing some > nice work. I would like to request a few things before I make a proper > review of the logic of the code: > > 1) Please try to polish the commits so that they are easier to review: > squash unnecessary "fix" commits into others, so that there are no > code removals in new code, and in general try to adopt a commit > history that looks like: > > * Implement Foo > * Implement Bar > > ...without anything else. This will make it much easier for me to review. > > 2) On coding style: Please go over the code again and try to use the > coding style of the rest of tp-qt. You have done it for most of the > code, but not everywhere. For example: > > * void BaseChannel::setInitiatorID( const QString& initiatorID ) >-> void BaseChannel::setInitiatorID(const QString &initiatorID) > * #ifndef BASECHANNEL_H > -> #ifndef _TelepathyQt_base_channel_h_HEADER_GUARD_ > > 3) Your code is missing a copyright & license. It obviously cannot be > merged like this. > > 4) To follow the style of the rest of tp-qt, pretty headers should > exist for every class, not just for BaseChannel. > > This is all for now. Thanks a lot for your work and I hope to see it > merged. > > Regards, > George > ___ telepathy mailing list telepathy@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/telepathy
[Telepathy] ANNOUNCE: Empathy 3.8.0
Empathy 3.8.0 is now available for download from: http://download.gnome.org/sources/empathy/3.8/ 2e31241929d2de09aed3b1d9688058c9 empathy-3.8.0.tar.xz What is it? === Empathy is a messaging program which supports text, voice, and video chat and file transfers over many different protocols. Empathy is the default chat client in GNOME, and is based on the Telepathy framework, making it easier for other GNOME applications to integrate collaboration functionality. You can visit the project web site: http://live.gnome.org/Empathy What's New? === Bugs fixed: - Fixed #604975, Lack of overview for those new to IM (Aruna) Translations: - Updated el Translation (Piotr Drąg) - Updated fa Translation (Arash Mousavi) - Updated fi Translation (Ville-Pekka Vainio) - Updated gu Translation (Sweta Kothari) - Updated hi Translation (Rajesh Ranjan) - Updated mr Translation (Sandeep Sheshrao Shedmake) - Updated or Translation (ManojKumar Giri) - Updated ru Translation (Yuri Myasoedov) - Updated sk Translation (Pavol Klačanský) - Updated sl Translation (Andrej Žnidaršič) - Updated ta Translation (Dr.T.Vasudevan) - Updated te Translation (Krishnababu Krothapalli) Documentation translations: - Updated fr Documentation translation (Alexandre Franke) - Updated hu Documentation translation (Gabor Kelemen) 25 March 2013 Empathy team ___ telepathy mailing list telepathy@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/telepathy
Re: [Telepathy] Work on telepathy-qt service
On Mon, Mar 11, 2013 at 6:34 AM, Matthias Gehre wrote: > Hi, > > I have been working on a connection manager for whatsapp [1] using > telepathy-qt. As the telepathy-qt service part is lacking, I implemented [2] > the interfaces I needed (see appendix). My changes mainly add code and only > touch code in the service part (which is not built by default), so I hope > this can be merged. I would like your opinions on what how to improve the > code before merging. > > The branch also includes three generic commits: One which replaces > QString::toAscii with QString::toLatin1, because the former is no longer > available in QT 5. On which does the same for QWeakPointer -> QPointer. > > And a commit that makes cmake find QT5. Do not merge that commit, it will > break QT4. Some better way has to be found there. > > Best wishes, > Matthias > > Implemented interfaces: > BaseChannel, > BaseChannelTextType, > BaseChannelMessagesInterface, > BaseChannelServerAuthenticationType, > BaseChannelCaptchaAuthenticationInterface, > BaseConnectionRequestsInterface > BaseConnectionContactsInterface > BaseConnectionSimplePresenceInterface > BaseConnectionContactListInterface > BaseConnectionAddressingInterface > > > [1] https://github.com/mgehre/telepathy-whosthere > [2] https://github.com/mgehre/telepathy-qt > Hello, Sorry for the delayed reply. It is great to see somebody is working on this feature of tp-qt and that there is at last a connection manager implemented with Qt :) I had a quick look at your branch and it looks like you are doing some nice work. I would like to request a few things before I make a proper review of the logic of the code: 1) Please try to polish the commits so that they are easier to review: squash unnecessary "fix" commits into others, so that there are no code removals in new code, and in general try to adopt a commit history that looks like: * Implement Foo * Implement Bar ...without anything else. This will make it much easier for me to review. 2) On coding style: Please go over the code again and try to use the coding style of the rest of tp-qt. You have done it for most of the code, but not everywhere. For example: * void BaseChannel::setInitiatorID( const QString& initiatorID ) -> void BaseChannel::setInitiatorID(const QString &initiatorID) * #ifndef BASECHANNEL_H -> #ifndef _TelepathyQt_base_channel_h_HEADER_GUARD_ 3) Your code is missing a copyright & license. It obviously cannot be merged like this. 4) To follow the style of the rest of tp-qt, pretty headers should exist for every class, not just for BaseChannel. This is all for now. Thanks a lot for your work and I hope to see it merged. Regards, George ___ telepathy mailing list telepathy@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/telepathy
