270903 - calICalendar updates

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Updates to calICalendar.idl as per irc discussion; some cleanup on the item
getters and some other bits.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: calICalendar.idl.patches

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 166532 [details] [diff] [review]
calICalendar.idl.patches

>+   * parameters:
>+   *
>+   * - aOperationType: calIOperationListener::ADD
>+   * - aId: the ID of the newly added item
>+   * - aStatus: nsresult code indicating the result of operation
>+   * - aDetail: the calIItemBase corresponding to the immutable
>+   *            version of the newly added item

I'd suggest dropping the above documentation.  This stuff is already documented
in calIOperationListener, and keeping one piece of documentation up-to-date is
generally hard enough; keeping two up-to-date and in-sync is asking for
trouble, I think.  Just referring the reader to calIOperationListener should be
sufficient, I think.  Same with the analogous commentary in the other methods
here.

>+   * If the generation of the given aItem does not match the generation
>+   * of the internal item (indicating that someone else modified the
>+   * item), onOperationComplete is called with a status of NS_ERROR_XXXXX
>+   * and aDetail is set to the latest-version internal immutable item.

Seems like there is some overlap in funcationality between lastModifiedTime and
generation.  Do we actually need both?

Also, please add your name to the license boilerplate.

Comment 3

[Mike Shaver (:shaver emeritus)]

Those comments don't duplicate the calIOperationListener documentation though,
which latter comments don't tell you, for example, what the meaning of aDetail
is in response to a given operation.

Comment 4

[Mike Shaver (:shaver emeritus)]

Yes, we need both a sync generation and lastModifiedTime -- relying on
globally-synchronized clocks to detect lost updates is a recipe for purest pain.

(Even, it turns out, on atomic-clock-sync'd-clusters.)

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to comment #3)
> Those comments don't duplicate the calIOperationListener documentation though,
> which latter comments don't tell you, for example, what the meaning of aDetail
> is in response to a given operation.

Ah, true.  Better keep that and any other non-duplicated info, then.

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 166532 [details] [diff] [review]
calICalendar.idl.patches

>Index: calICalendar.idl
>+   * @param aCount      Maximum number of items to return.  
What if i want all the items? Pass 0?

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: 270903-calicalendar-1.patch

Updated as per comments; removed duplicate documentation about various params. 
Also added note about aCount (set to 0 for unbounded query).

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 166801 [details] [diff] [review]
270903-calicalendar-1.patch

r=dmose

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: 270903-calicalendar-2.patch

Moved the aStatus parameter to the two listener callbacks to always be the
first param, for consistency.

Comment 10

[Simon Paquet [:sipaq]]

Comment on attachment 166532 [details] [diff] [review]
calICalendar.idl.patches

patch is obsolete, removing review request.

Comment 11

[Reed Loden [:reed]]

The bugspam monkeys have been set free and are feeding on Calendar :: General. Be afraid for your sanity!