[Bug 28197] Account.I.External: support for immutable accounts in Mission Control

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=28197

--- Comment #10 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-06-17 10:16:39 PDT ---
Each property should say whether it can change, and if so, how you get
notified. I infer that the intention is (and here's how I'd phrase it):

StorageProvider: This property cannot change once an Account has been created.

StorageIdentifier: This property cannot change once an Account has been
created.

StorageRestrictions: This property cannot change once an Account has been
created.

StorageSpecificInformation: Whether values in this map can change is defined by
the StorageProvider. There is no change notification, so the property must be
re-retrieved using Get or GetAll if an up to date value is needed.

> +        Dbus namespaced name of the plugin in the AccountManager implementation

It's spelled D-Bus, and I'd phrase this more like: The name of the account
storage implementation, which SHOULD start with a reversed domain name in the
same way as D-Bus interface names.

> +      <tp:docstring>
> +        Unique identification of the account within the backends Storage.
> +        What's stored in the variant is specific to the storage implementation.
> +
> +        <tp:rationale>

... within the storage backend. The contents of the variant are defined by the
<tp:member-ref>StorageProvider</tp:member-ref>.

Spec-lint: Please put <tp:docstring xmlns="http://www.w3.org/1999/xhtml"> on
any docstring with multiple paragraphs (rationale counts as a <div>), and put
<p> around each paragraph (including inside the <tp:rationale>).

> +          identifying an account, some might use an integer others a string.

... an account, typically an integer or a string.

> +    <property name="StorageSpecificInformation"
> +      <tp:docstring>
> +        Map containing information specific to one Storage system. This map
> +        contains extra information specific to the storage method. The possible
> +        keys depend solely on the used Storage method and won't be further
> +        defined by this spec.
> +      </tp:docstring>

This should also say that the AM won't use this information, and should have a
rationale. I'd phrase this as:

<p>Map containing information specific to the storage backend. The keys and the
types of their values are defined by the
<tp:member-ref>StorageProvider</tp:member-ref>, and are not interpreted by the
AccountManager implementation.</p>

<tp:rationale>
  <p>This can be used to provide additional hints to user interfaces aware of a
specific storage provider, without requiring those user interfaces to use the
<tp:member-ref>StorageIdentifier</tp:member-ref> to query the storage provider
directly.</p>
</tp:rationale>

That rationale seems a bit weak, though... apart from fewer round-trips, that
property just gives us the ability for two cooperating backends to agree on a
hint name.

> +    <tp:flags name="Storage_Restriction_Flags" value-prefix="Storage_Flag"

Either change the value-prefix to Storage_Restriction_Flag or the name to
Storage_Flags, please. I think I prefer the former; or possibly it could be
Storage_Restriction and Storage_Restrictions?

I'm not sure that I like the names starting with "Immutable" - the
enabledness/parameters/presence can presumably change, just not via Telepathy?
We generally use Immutable to mean "can't change under any circumstances".

Perhaps Cannot_Set_Parameters, Cannot_Set_Enabled, Cannot_Set_Presence?

Spec-lint: I'd prefer a blank line between each <tp:flag> and the next.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.