Bug 70382 - consider merging daemons' extension interfaces into tp-spec
Summary: consider merging daemons' extension interfaces into tp-spec
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-spec (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords: patch
Depends on: 25147 26172 26609 33101 33485 37380 69600
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-11 16:23 UTC by Simon McVittie
Modified: 2014-02-07 14:33 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
[spec master] Turn Connection.FUTURE into Conn.I.Sidecars1 (3.21 KB, patch)
2013-10-11 16:33 UTC, Simon McVittie
Details | Splinter Review
[spec master] Add Conn.I.Decloak1, from Gabble (6.11 KB, patch)
2013-10-11 16:36 UTC, Simon McVittie
Details | Splinter Review
[salut master] constants, servicetest: sync from Gabble (23.32 KB, patch)
2014-02-07 13:09 UTC, Simon McVittie
Details | Splinter Review
[salut master] Use telepathy-glib for Sidecars1 interface (11.64 KB, patch)
2014-02-07 13:10 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2013-10-11 16:23:26 UTC
Various CMs, and MC, have various surplus interfaces that never got merged.
Comment 1 Simon McVittie 2013-10-11 16:32:54 UTC
Candidates:

Mission Control (Bug #69600)
============================

* Conditions: let's just delete it, Bug #69600
* Hidden: Bug #33101
* CD.I.Messages: Bug #37380
* A.I.ExternalPasswordStorage, CM.I.AccountStorage: merged, we just need to do the tp-spec -> tp-glib -> MC dance

Gabble & Salut
==============

* Connection_Future: turn into Conn.I.Sidecars1
* OLPC_*: um... no. If OLPC/Sugar still use these, they should really be a sidecar anyway.
* Salut_Plugin_Test, Gabble_Plugin_*: leave in the CMs
* Channel_Type_FileTransfer_Future.FileCollection: Bug #26609
* Decloak: not sure what to do about this one... it could be core spec

Idle
====

* Renaming: Bug #25147
Comment 2 Simon McVittie 2013-10-11 16:33:50 UTC
Created attachment 87473 [details] [review]
[spec master] Turn Connection.FUTURE into Conn.I.Sidecars1

If something has been implemented without changes for 4 years, but
has not been declared core functionality in that time, then it's
clearly an optional interface.

This is a simple rename: no content changes other than the name and
the <tp:added>. Connection.FUTURE was also identical to the version
in Gabble and Salut, apart from the <tp:added>.

---

Still to do: make Gabble and Salut use this version, via telepathy-glib.
Comment 3 Simon McVittie 2013-10-11 16:36:22 UTC
Created attachment 87475 [details] [review]
[spec master] Add Conn.I.Decloak1, from Gabble

---

Not sure whether to merge this one or not... it's pretty XMPP-specific, but then, Conn.I.Cellular is protocol-specific and we still spec'd that one centrally.
Comment 4 Guillaume Desmottes 2013-10-12 14:34:53 UTC
Comment on attachment 87473 [details] [review]
[spec master] Turn Connection.FUTURE into Conn.I.Sidecars1

Review of attachment 87473 [details] [review]:
-----------------------------------------------------------------

++

I'll port my Gabble next branch once we have a tp-glib release with it.
Comment 5 Guillaume Desmottes 2013-10-12 14:35:48 UTC
Comment on attachment 87475 [details] [review]
[spec master] Add Conn.I.Decloak1, from Gabble

Review of attachment 87475 [details] [review]:
-----------------------------------------------------------------

I'm not sure either about this one. Is it actually used these days?
Comment 6 Simon McVittie 2013-10-14 15:38:31 UTC
(In reply to comment #5)
> [spec master] Add Conn.I.Decloak1, from Gabble
> I'm not sure either about this one. Is it actually used these days?

Let's take this to Bug #26172 now that I've realised that bug exists.
Comment 7 Simon McVittie 2013-10-14 17:06:19 UTC
Comment on attachment 87473 [details] [review]
[spec master] Turn Connection.FUTURE into Conn.I.Sidecars1

In spec master. Still to do: release in spec 0.27.3, release in tp-glib 0.23.0, change Gabble and Salut.
Comment 8 Simon McVittie 2013-10-15 12:47:34 UTC
(In reply to comment #1)
> * A.I.ExternalPasswordStorage, CM.I.AccountStorage: merged, we just need to
> do the tp-spec -> tp-glib -> MC dance

Actually, no, they're still marked as DRAFT in the spec. Let's just undraft them: the implementation in MC isn't great, but it'll do.
Comment 9 Simon McVittie 2013-10-15 14:24:29 UTC
(In reply to comment #8)
> Actually, no, they're still marked as DRAFT in the spec. Let's just undraft
> them: the implementation in MC isn't great, but it'll do.

I noticed they don't have any regression test coverage, started writing a test, ran into "ugh, how much of a CM do I need to implement?" and lost interest for now. Bug #34640, anyway.
Comment 10 Guillaume Desmottes 2013-10-21 13:10:08 UTC
Empathy
=======

* Channel.Interface.CredentialsStorage.DRAFT: still a DRAFT in the spec. I'd be tempted to drop it.
* Channel.Type.ServerTLSConnection: I'm about to remove it (bgo#tls-630896) as bug #30460 has been merged
* Connection.Interface.Renaming: bug #25147
* Logger.DRAFT: keep as a DRAFT for now and rename to Logger1 in next?
Comment 11 Simon McVittie 2013-10-23 13:21:50 UTC
(In reply to comment #10)
> * Channel.Interface.CredentialsStorage.DRAFT: still a DRAFT in the spec.

If there's an implementation that proves it makes sense, we can stick a 1 on the end and ship it, or if not, we should get rid of it. Bug #33485 was closed before any of this stuff was actually undrafted. We should stop doing that :-(

As far as I understand it, Chan.I.CredentialsStorage, A.I.ExternalPasswordStorage and CM.I.AccountStorage are all the same high-level feature? We should open a bug for them, or perhaps reopen Bug #33485.

> * Channel.Type.ServerTLSConnection: I'm about to remove it (bgo#tls-630896)
> as bug #30460 has been merged
> * Connection.Interface.Renaming: bug #25147

Great.

> * Logger.DRAFT: keep as a DRAFT for now and rename to Logger1 in next?

Yeah :-(

If its methods are stable, we should incorporate it into the spec proper. If not, we should give it a name that indicates that it's an implementation detail of a C library (x.Private.y seems to be the GLib convention).

The Logger's bus name should probably be versioned, too.
Comment 12 Simon McVittie 2013-11-04 16:55:05 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > * Channel.Interface.CredentialsStorage.DRAFT: still a DRAFT in the spec.
> 
> If there's an implementation that proves it makes sense, we can stick a 1 on
> the end and ship it, or if not, we should get rid of it. Bug #33485 was
> closed before any of this stuff was actually undrafted. We should stop doing
> that :-(

I reopened it.
Comment 13 Simon McVittie 2014-01-30 13:40:56 UTC
Progress report:

done
====

* deleted: Conditions, Hidden
* undrafted: CD.I.Messages, Conn.I.Sidecars1, Renaming, FileCollection

won't be merged
===============

* Gabble and Salut's plugin test interfaces
* Gabble's gateway-registration plugin

nearly done
===========

* about to be deleted: A.I.ExternalPasswordStorage, CM.I.AccountStorage

unresolved
==========

* Chan.I.CredentialsStorage in Empathy (delete?)
* Gabble Decloak interface (Bug #26172; delete?)
* OLPC interfaces (delete?)
* Logger (undraft?)
Comment 14 Guillaume Desmottes 2014-01-30 14:52:47 UTC
(In reply to comment #13)
> * Chan.I.CredentialsStorage in Empathy (delete?)

agreed.

> * Gabble Decloak interface (Bug #26172; delete?)

Fine with me.

> * OLPC interfaces (delete?)

I removed them in Gabble and Salut next so yeah, let's delete them.

> * Logger (undraft?)

Sounds good.
Comment 15 Guillaume Desmottes 2014-02-04 12:01:28 UTC
(In reply to comment #14)
> (In reply to comment #13)
> > * Chan.I.CredentialsStorage in Empathy (delete?)
> 
> agreed.

http://cgit.collabora.com/git/user/cassidy/empathy/log/?h=next-remove-cred-storage
Comment 16 Simon McVittie 2014-02-04 12:20:02 UTC
> http://cgit.collabora.com/git/user/cassidy/empathy/log/?h=next-remove-cred-storage

Ship it

(Also, please use telepathy-glib for Renaming)
Comment 17 Guillaume Desmottes 2014-02-04 12:43:53 UTC
(In reply to comment #16)
> > http://cgit.collabora.com/git/user/cassidy/empathy/log/?h=next-remove-cred-storage
> 
> Ship it

Merged.

> (Also, please use telepathy-glib for Renaming)

I did it in master first; then I'll merge it back to next:
http://cgit.collabora.com/git/user/cassidy/empathy/log/?h=renaming
Comment 18 Simon McVittie 2014-02-04 12:50:04 UTC
(In reply to comment #17)
> I did it in master first; then I'll merge it back to next:
> http://cgit.collabora.com/git/user/cassidy/empathy/log/?h=renaming

Looks fine
Comment 19 Guillaume Desmottes 2014-02-04 13:28:11 UTC
(In reply to comment #14)
> I removed them in Gabble and Salut next so yeah, let's delete them.

Actually I just did Gabble so far:
http://cgit.collabora.com/git/user/cassidy/telepathy-salut/log/?h=next-olpc
Comment 20 Simon McVittie 2014-02-04 13:31:23 UTC
(In reply to comment #19)
> Actually I just did Gabble so far:
> http://cgit.collabora.com/git/user/cassidy/telepathy-salut/log/?h=next-olpc

++
Comment 21 Guillaume Desmottes 2014-02-04 14:10:01 UTC
(In reply to comment #20)
> (In reply to comment #19)
> > Actually I just did Gabble so far:
> > http://cgit.collabora.com/git/user/cassidy/telepathy-salut/log/?h=next-olpc
> 
> ++

merged.


(In reply to comment #18)
> (In reply to comment #17)
> > I did it in master first; then I'll merge it back to next:
> > http://cgit.collabora.com/git/user/cassidy/empathy/log/?h=renaming
> 
> Looks fine

Merged to master and next (just appened '1' to iface name).
Comment 22 Guillaume Desmottes 2014-02-04 15:00:28 UTC
(In reply to comment #13)
> * Logger (undraft?)

http://cgit.collabora.com/git/user/cassidy/telepathy-spec/log/?h=next-logger

I'll port tp-glib and empathy.
Comment 24 Simon McVittie 2014-02-05 14:33:47 UTC
++
Comment 25 Guillaume Desmottes 2014-02-05 14:37:27 UTC
Merged; closing \o/
Comment 26 Simon McVittie 2014-02-07 13:09:19 UTC
Salut still uses Connection.Future for its sidecars.
Comment 27 Simon McVittie 2014-02-07 13:09:54 UTC
Created attachment 93602 [details] [review]
[salut master] constants, servicetest: sync from Gabble
Comment 28 Simon McVittie 2014-02-07 13:10:09 UTC
Created attachment 93603 [details] [review]
[salut master] Use telepathy-glib for Sidecars1 interface
Comment 29 Simon McVittie 2014-02-07 13:17:09 UTC
(In reply to comment #28)
> [salut master] Use telepathy-glib for Sidecars1 interface

This merges fairly nicely into next, resolving the conflicts in tests/twisted/constants.py and tests/twisted/servicetest.py by copying them from gabble/next.
Comment 30 Guillaume Desmottes 2014-02-07 13:26:54 UTC
Comment on attachment 93602 [details] [review]
[salut master] constants, servicetest: sync from Gabble

Review of attachment 93602 [details] [review]:
-----------------------------------------------------------------

++
Comment 31 Guillaume Desmottes 2014-02-07 13:27:20 UTC
Comment on attachment 93603 [details] [review]
[salut master] Use telepathy-glib for Sidecars1 interface

Review of attachment 93603 [details] [review]:
-----------------------------------------------------------------

++
Comment 32 Simon McVittie 2014-02-07 14:18:08 UTC
Thanks, merged. Non-trivial merge into next:

http://cgit.freedesktop.org/~smcv/telepathy-salut/commit?h=next
Comment 33 Guillaume Desmottes 2014-02-07 14:30:59 UTC
(In reply to comment #32)
> Thanks, merged. Non-trivial merge into next:
> 
> http://cgit.freedesktop.org/~smcv/telepathy-salut/commit?h=next

Looks good.
Comment 34 Simon McVittie 2014-02-07 14:33:30 UTC
Fixed again.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.