[Telepathy] Work on telepathy-qt service

[Matthias Gehre]

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 <kiagiadakis.george at gmail.com>

> On Mon, Mar 11, 2013 at 6:34 AM, Matthias Gehre <M.Gehre at gmx.de> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/telepathy/attachments/20130325/dcdb97f7/attachment.html>