[Telepathy] Work on telepathy-qt service

[George Kiagiadakis]

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