Summary: | Bind Account.ChangingPresence | ||
---|---|---|---|
Product: | Telepathy | Reporter: | Simon McVittie <smcv> |
Component: | tp-qt | Assignee: | Andre Moreira Magalhaes <andrunko> |
Status: | RESOLVED FIXED | QA Contact: | Telepathy bugs list <telepathy-bugs> |
Severity: | enhancement | ||
Priority: | medium | CC: | jan.oelze |
Version: | unspecified | ||
Hardware: | Other | ||
OS: | All | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Bug Depends on: | 26752, 28536 | ||
Bug Blocks: |
Description
Simon McVittie
2010-05-07 07:26:24 UTC
Minor changes needed, see <https://bugs.freedesktop.org/show_bug.cgi?id=26752#c14> A spec release is also needed. In reply to https://bugs.freedesktop.org/show_bug.cgi?id=26752#c14 > No, I was complaining about the *signal*'s documentation. Compare: > > // getter > bool Tp::Account::isChangingPresence(void) const; > > // signal > void Tp::Account::changingPresence(bool); > > This is more subtle than it ought to be :-) True I missed that. will fix. > Would it perhaps be clearer if we had two signals, like this? > > // getter > bool Tp::Account::isChangingPresence(void) const; > > // signal (part 1) > void Tp::Account::presenceChangeStarted(void); > > // signal (part 2) > void Tp::Account::presenceChangeFinished(void); Hmm I am not sure about this, it surely would be more clear, but having one signal I believe it's easier to implement. We can have on signal handler to start/stop blinking the UI for example. Not sure here (In reply to comment #2) > Hmm I am not sure about this, it surely would be more clear, but having one > signal I believe it's easier to implement. We can have on signal handler to > start/stop blinking the UI for example. Not sure here Yeah, I'm not sure about this either - normally I'd vote for a smaller and more consistent API (i.e. one signal with a boolean argument, like you already implemented), but it does lead to confusingly similar method and signal names here, so the question is whether an larger and less consistent API is a worthwhile price for better clarity. Keep it in the back of your mind for now, and we can make a final decision after the spec gets released? Updated branch to fix the issue with the signal documentation and depend on the new spec. Chose to keep using changingPresence() for the signal name, I think it's the best way here, but I am opened for suggestions. This looks good to me. Merged upstream. It will be in next release 0.3.5 |
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.