[Telepathy] Work on telepathy-qt service

George Kiagiadakis kiagiadakis.george at gmail.com
Mon Mar 25 03:10:20 PDT 2013

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


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


More information about the telepathy mailing list