Bug 27997

There are actually three branches here.

basic-protocol-objects adds a TpBaseProtocol base class. It doesn't depend on new codegen, so if we're confident that this code is "the right shape", we could merge it at any time.

protocol-export and protocol-export-2 add the Protocol codegen (which includes a stub TpProtocol class) and turn TpBaseProtocol into a full Protocol object.

See also Bug #27998, which adds the rest of TpProtocol.

Review+ from ae8a03 to 64eeb7

(In reply to comment #2)
> Review+ from ae8a03 to 64eeb7

Thanks. (For the record: this covers all of basic-protocol-objects, protocol-export and protocol-export-2.)

Any thoughts on the spec?

*** Bug 27998 has been marked as a duplicate of this bug. ***

Over on Bug #27998, Andre wrote:
> Rebased with upstream master and fixed some issues with tests+conflicts in docs
> mainly.
>
> The branch is at
> http://git.collabora.co.uk/?p=user/andrunko/telepathy-glib.git;a=shortlog;h=refs/heads/protocol

I've got rid of my branches in favour of this one, but in future I'd much prefer it if you didn't rebase other people's branches - it makes it difficult to work out what has changed, and introduces a lot of commits which would never actually have worked or even compiled.

A non-trivial merge of master into the branch, or the branch into a copy of master, with appropriate changes to make it compile and work again, would have preserved the history, and preserved the general principle that after each commit, the code should be (at least mostly) usable.

> Just one thing I missed in the review. The methods on Protocol
> (IdentifyAccount, NormalizeContact) are not implemented in client code, should
> they be? I didn't implement them in tp-qt4 either.

Do you mean that generated tp_cli functions are missing, or that high-level wrappers are missing?

As long as we have the generated tp_cli functions, I think we can defer high-level wrappers until a bit later, although we should open a bug for them so they don't get lost.

> I am using this rebased branch as requirement for tp-qt4 tests for Protocol
> objects, as I need the changed echo2 and the addition of protocol in
> ConnectionManager, ...

I've added a commit to fix the documentation, which wasn't merged quite right.

(In reply to comment #5)
> Over on Bug #27998, Andre wrote:
> > Rebased with upstream master and fixed some issues with tests+conflicts in docs
> > mainly.
> >
> > The branch is at
> > http://git.collabora.co.uk/?p=user/andrunko/telepathy-glib.git;a=shortlog;h=refs/heads/protocol
> 
> I've got rid of my branches in favour of this one, but in future I'd much
> prefer it if you didn't rebase other people's branches - it makes it difficult
> to work out what has changed, and introduces a lot of commits which would never
> actually have worked or even compiled.
> 
> A non-trivial merge of master into the branch, or the branch into a copy of
> master, with appropriate changes to make it compile and work again, would have
> preserved the history, and preserved the general principle that after each
> commit, the code should be (at least mostly) usable.
Sure, I will keep that in mind next time. I just wanted to have something working so I could work on tp-qt4 tests for Protocol easily.

> > Just one thing I missed in the review. The methods on Protocol
> > (IdentifyAccount, NormalizeContact) are not implemented in client code, should
> > they be? I didn't implement them in tp-qt4 either.
> 
> Do you mean that generated tp_cli functions are missing, or that high-level
> wrappers are missing?
> 
> As long as we have the generated tp_cli functions, I think we can defer
> high-level wrappers until a bit later, although we should open a bug for them
> so they don't get lost.
The high-level wrappers are missing, low-level methods are there. So I keep r+ here and we should open bugs to implement the missing high-level methods later. Same for tp-qt4 impl found at andrunko/cm-protocol that does not wrap these methods.

The branch smcv/protocol2 makes some changes I found were necessary while porting Haze to use TpBaseProtocol (Bug #29078).

Looks good.

... and again, making some changes suggested by wjt on Bug #29078.

Reviewed by andrunko and ptlo up to 4d74feb906b7f9e37238a9d89245eeaaf1a0ffed.

Fixed in git for 0.11.11. Thanks, everyone!