Bug 38852

Summary:	Call: Name methods like methods and signals like signals
Product:	Telepathy	Reporter:	David Laban <david.laban>
Component:	tp-spec	Assignee:	Telepathy bugs list <telepathy-bugs>
Status:	RESOLVED FIXED	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	normal
Priority:	medium	CC:	david.laban, olivier.crete
Version:	git master	Keywords:	patch
Hardware:	Other
OS:	All
Whiteboard:	Call
i915 platform:		i915 features:

Description David Laban 2011-06-30 13:26:43 UTC

The StreamedMedia api is embarrassing. Let's not make the same mistake in Call

git grep '<method' spec/*Call* shows the following questionable names:

spec/Call_Content_Interface_Media.xml:    <method name="Error"
spec/Call_Stream_Interface_Media.xml:    <method name="CandidatesPrepared"
spec/Call_Stream_Interface_Media.xml:    <method name="Error"

git grep '<signal' spec/*Call* reveals only:

spec/Call_Stream_Interface_Media.xml:    <signal name="PleaseRestartICE"

I plan to change PleaseRestartICE to ICERestartRequested (and have a boolean ICERestartPending for state recovery) as part of #35012 though, so no need to worry about that one.

CandidatesPrepared could be called FinishInitialCandidateBatch, but that's a horrible name. As someone who has been screwed both by failing to call this method in sip and failing to flush sockets in unix, I'm tempted to call it FlushCandidates. Thoughts?

The Error methods could both be called RemoveWithError.

Comment 1 Olivier Crête 2011-06-30 15:09:03 UTC

Fail() instead of Error() ?

InitialCandidatesDone() ?

I don't think ICERestartPending is required.. Since if you've lost your state, you'll have to restart anyway

Comment 2 David Laban 2011-07-01 17:32:34 UTC

(In reply to comment #1)
> Fail() instead of Error() ?
> 
I don't think we're going to beat that for conciseness.

> InitialCandidatesDone() ?
> 
Still sounds like a signal name. Going for FlushInitialCandidates.

http://cgit.collabora.com/git/user/alsuren/telepathy-spec.git/log/?h=call-trivia-38852 is ready for review.

Comment 3 Olivier Crête 2011-07-02 15:03:39 UTC

Flush sounds like your throwing them away!.. And it's a signal really

Comment 4 David Laban 2011-07-05 12:15:30 UTC

(In reply to comment #3)
> Flush sounds like your throwing them away!.. And it's a signal really

The name was inspired by fflush. "CandidatesPrepared" only sounds like a signal (past tense) because it was copy-pasted from the (inside-out-sounding) StreamedMedia API. It is a method, which will cause rakia to do something important (but may be ignored by gabble). It should therefore use the imperative mood.

I'm going to brainstorm a load of other possible names. Pick the one you like best.

FinishInitialCandidates()

SendInitialCandidates() # may be a lie if we don't have codecs yet.

DoCandidatesDance()

ProcessInitialCandidates()

ProcessCandidates() # Probably the most honest name.

Note that the purpose of this method is so that the CM can select its RawUDP candidate. If we just created a ProvideRawUDPCandidate() it would make the entire process more explicit.

Comment 5 Olivier Crête 2011-07-05 12:21:22 UTC

The propose of this method is not just selecting the rawudp candidate. In SIP, you need to wait for it before you send out any the SDP offer with all the candidates.

The least bad in your list is FinishInitialCandidates().. but really it is a signal, despite being a method call ...

Comment 6 David Laban 2011-07-05 12:46:45 UTC

Updated http://cgit.collabora.com/git/user/alsuren/telepathy-spec.git/log/?h=call-trivia-38852 to reflect this discussion.

Comment 7 Olivier Crête 2011-07-05 12:56:32 UTC

++
SHIP IT!

Comment 8 David Laban 2011-07-05 13:11:18 UTC

merged to alsuren/call.

Comment 9 David Laban 2011-07-19 12:47:59 UTC

merged to master

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.