Bug 38852 - Call: Name methods like methods and signals like signals
Summary: Call: Name methods like methods and signals like signals
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-spec (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL:
Whiteboard: Call
Keywords: patch
Depends on:
Blocks:
 
Reported: 2011-06-30 13:26 UTC by David Laban
Modified: 2011-07-19 12:47 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments

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.