On Mon, Jan 14, 2013 at 05:45:45PM -0500, Saggi Mizrahi wrote:
> 
> 
> ----- Original Message -----
> > From: "Adam Litke" <a...@us.ibm.com>
> > To: "Saggi Mizrahi" <smizr...@redhat.com>
> > Cc: vdsm-devel@lists.fedorahosted.org, "Vinzenz Feenstra" 
> > <vfeen...@redhat.com>
> > Sent: Monday, January 14, 2013 5:21:41 PM
> > Subject: Re: [vdsm] API Documentation & Since tag
> > 
> > On Mon, Jan 14, 2013 at 12:37:57PM -0500, Saggi Mizrahi wrote:
> > > 
> > > 
> > > ----- Original Message -----
> > > > From: "Adam Litke" <a...@us.ibm.com>
> > > > To: "Vinzenz Feenstra" <vfeen...@redhat.com>
> > > > Cc: vdsm-devel@lists.fedorahosted.org
> > > > Sent: Friday, January 11, 2013 9:03:19 AM
> > > > Subject: Re: [vdsm] API Documentation & Since tag
> > > > 
> > > > On Fri, Jan 11, 2013 at 10:19:45AM +0100, Vinzenz Feenstra wrote:
> > > > > Hi everyone,
> > > > > 
> > > > > We are currently documenting the API in vdsmapi-schema.json
> > > > > I noticed that we have there documented when a certain element
> > > > > newly
> > > > > is introduced using the 'Since' tag.
> > > > > However I also noticed that we are not documenting when a field
> > > > > was
> > > > > newly added, nor do we update the 'since' tag.
> > > > > 
> > > > > We should start documenting in what version we've introduced a
> > > > > field.
> > > > > A suggestion by saggi was to add to the comment for example:
> > > > > @since: 4.10.3
> > > > > 
> > > > > What is your point of view on this?
> > > > 
> > > > I do think it's a good idea to add this information.  How about
> > > > supporting
> > > > multiple Since lines in the comment like the following made up
> > > > example:
> > > > 
> > > > ##
> > > > # @FenceNodePowerStatus:
> > > > #
> > > > # Indicates the power state of a remote host.
> > > > #
> > > > # @on:        The remote host is powered on
> > > > #
> > > > # @off:       The remote host is powered off
> > > > #
> > > > # @unknown:   The power status is not known
> > > > #
> > > > # @sentient:  The host is alive and powered by its own metabolism
> > > > #
> > > > # Since: 4.10.0 - @FenceNodePowerStatus
> > > > # Since: 10.2.0 - @sentient
> > > > ##
> > > I don't like the fact that both lines don't point to the same type
> > > of token.
> > > I also don't like that it's a repeat of the type names and field
> > > names.
> > > 
> > > I prefer Vinzenz original suggestion (on IRC) of moving the "Since"
> > > token up and then
> > > have it be a state.  It also makes discerning what entities you can
> > > use up to a
> > > certain version easier if you make sure to keep them sorted.
> > > 
> > > We can do this because the order of the fields and availability is
> > > undetermined (unlike real structs).
> > 
> > That is not correct.  These structures are parsed into an OrderedDict
> > and the
> > ordering is important (especially for languages like C which might
> > use real
> > structs).
> The "wire" format, json, ignores the ordering, further more, for
> languages like C we can't use actual structs because then we have
> to bump a major version every time we add a field as the
> sizeof(struct Foo) changed
> > 
> > > 
> > > ##
> > > # @FenceNodePowerStatus:
> > > #
> > > # Indicates the power state of a remote host.
> > > #
> > > # Since: 4.10.0
> > > #
> > > # @on:        The remote host is powered on
> > > #
> > > # @off:       The remote host is powered off
> > > #
> > > # @unknown:   The power status is not known
> > > #
> > > # Since: 10.2.0
> > > #
> > > # @sentient:  The host is alive and powered by its own metabolism
> > > #
> > > ##
> > > 
> > > The problem though is that it makes since a property of the fields
> > > and not of
> > > the struct. This isn't that much of a problem as we can assume the
> > > earliest
> > > version is the time when the struct was introduced.
> > 
> > I don't like this any better than my suggestion.  Aside from the fact
> > that field
> > ordering is important (in the data structure itself), this spreads
> > the since
> > information throughout the comment rather than concentrating it in a
> > single
> > place.
> 
> Well, thinking about it, I don't understand why structs need to have a
> "Since" property anyway. Only verbs should have it. Structs are
> available (by inference) since the earliest call that produces them.
> 
> All fields in a struct are optional anyway. Old versions wouldn't try
> and access them, new clients should always assume these fields may
> not be returned anyway.

All _newly_added_ fields must be optional.  Fields that are part of the original
definition of the type can be required fields.  This reminds me that we will
need to audit the schema for fields that can be made optional.  For example,
when creating Vm*Device objects, the VmDeviceAddress member can be omitted.

-- 
Adam Litke <a...@us.ibm.com>
IBM Linux Technology Center

_______________________________________________
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel

Reply via email to