[Telepathy] fix checkValidProtocolName test in BaseConnection (etc.)

[Simon McVittie]

On 06/01/13 19:12, Maksim Melnikau wrote:
> [PATCH 1/3] fix checkValidProtocolName test in BaseConnection

This patch looks reasonable, but I'm out of touch with telepathy-qt, so
someone who is active on that project should review merge it. Please
consider opening a bug (bugs.freedesktop.org, product = Telepathy,
component = tp-qt) so it doesn't get lost.

The error message should probably also gain a space, so that if you use
an invalid protocol name like "###" it doesn't have a missing space like
"###is not a valid protocol name".

I think the need for this change demonstrates how much testing
BaseConnection has had... I'm disappointed that it was merged without a
regression test that would have caught this (and also the bug fixed by
your patch 2/3).

> [PATCH 2/3] fix busName and objectPath in BaseConnection

The same comments.

> [PATCH 3/3] add .(dot) at TP_QT_ACCOUNT_OBJECT_PATH_BASE end

No, this is an incompatible change (code that was correct before this
change would become incorrect) and should be rejected. The commit
message is also wrong, it talks about dots where in fact the relevant
character is a slash.

It would be useful to fix the documentation to note that you need to
append a slash before the CM name, though.

It is unfortunate if the various _BASE constants are not consistent, but
that can't be fixed in a compatible way, except by deprecating the whole
lot and adding a new set (with a new naming convention) that are
consistent. Whether that is worthwhile is up to the telepathy-qt
maintainers.

(For what it's worth, the equivalent constants in telepathy-glib
consistently do end with a separator.)

Regards,
    S