| Summary: | Protocol objects don't define a serialization for all their properties | ||
|---|---|---|---|
| Product: | Telepathy | Reporter: | Simon McVittie <smcv> |
| Component: | tp-spec | Assignee: | Simon McVittie <smcv> |
| Status: | RESOLVED FIXED | QA Contact: | Telepathy bugs list <telepathy-bugs> |
| Severity: | normal | ||
| Priority: | medium | CC: | senko.rasic |
| Version: | git master | Keywords: | patch |
| Hardware: | Other | ||
| OS: | All | ||
| URL: | http://git.collabora.co.uk/?p=user/smcv/telepathy-spec-smcv.git;a=shortlog;h=refs/heads/protocol-followup | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
|
Description
Simon McVittie
2010-07-21 05:35:17 UTC
Review of 9a71e0: +RequestableChannelClasses=text Shouldn't that be "RequestableChannelClasses=text;", since it's a string list? Also, AFAICT, there are no names associated with the channel classes, so the section names must be autogenerated. Of course, the names aren't actually used for anything except serialization here, but it'd be helpful if the spec could give guidance on how to generate them. We could e.g. use the channel type suffix and (a representation of) target handle type, to make the names somewhat human-readable, and append a counter to each name to ensure uniqueness. This is what I've implemented in gabble branch implementing Protocol support. The rest looks fine to me. (In reply to comment #1) > Review of 9a71e0: > > +RequestableChannelClasses=text > > Shouldn't that be "RequestableChannelClasses=text;", since it's a string list? Well spotted; there was also a pre-existing instance of the same error. Fixed in the updated branch. > Also, AFAICT, there are no names associated with the channel classes, so the > section names must be autogenerated. Of course, the names aren't actually used > for anything except serialization here, but it'd be helpful if the spec could > give guidance on how to generate them. I've explicitly said that they're not significant, and given one example of a mnemonic name, and one example of sequential names. Is that OK from your point of view? > We could e.g. use the channel type suffix and (a representation of) target > handle type, to make the names somewhat human-readable, and append a counter to > each name to ensure uniqueness. This is what I've implemented in gabble branch > implementing Protocol support. That sounds reasonable, but is an implementation detail of Gabble. I must admit that I was mostly assuming .manager files would be hand-written (the auto-generation that Gabble does prevents cross-compilation, which is a long-standing bug). (In reply to comment #2) > I've explicitly said that they're not significant, and given one example of a > mnemonic name, and one example of sequential names. Is that OK from your point > of view? Yeah, that looks fine to me. Overall ++ for the branch, merge away :) Fixed in git for 0.19.10 |
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.