196575 - Add comments to IDL interfaces

Status: VERIFIED FIXED

(Core Graveyard :: SQL, defect, P3)

Description

[Jan Varga [:janv]]

Comment 1

[Neil Deakin]

I started working on adding comments to the IDL files. I can submit a patch if
you like.

Comment 2

[Jan Varga [:janv]]

sure, that would be awesome!

Comment 3

[Neil Deakin]

Attachment: Add comments to IDL files

Comment 4

[Jan Varga [:janv]]

Comment on attachment 131806 [details] [diff] [review]
Add comments to IDL files

everything looks pretty good, I really appreciate this work!
although there's a little misunderstanding in the mozISqlResultEnumerator
interface

>+  /**
>+   * Sets the value at the specified column in the current row to its
>+   * original value. Changes are not committed until the copy method is
>+   * called.
>+   *
>+   * @param aColumnIndex   the column to modify
>+   */
>   void setDefault(in long aColumnIndex);

actually the default means the default column value for a table
create table foo (
  bar int default 1
)

so it will be processed like this:
insert into foo values(DEFAULT)
or
update foo set bar=DEFAULT

> 
>+  /**
>+   * Commits changes to the current row for a given column.
>+   *
>+   * @param aColumnIndex   the column to modify
>+   */
>   void copy(in long aColumnIndex);

This is not a commit, it just fills buffer for the current row with original
values. Usuful for updates.

> 
>+  /**
>+   * Sets the value at the specified column in the current row to a variant.
>+   * Changes are not committed until the copy method is called.

I'd say that changes are not commited until insertRow() or updateRow() is
called.
The same for other similar methods.

>+   *
>+   * @param aColumnIndex   the column to modify
>+   * @param aValue         the value to assign
>+   */
>   void setVariant(in long aColumnIndex, in nsIVariant aValue);

>+  /**
>+   * Sets the value of the cells in all columns in the current row to null.
>+   * Changes are not committed until the copyValues method is called.
>+   */
>   void setNullValues();

needs to be fixed too

> 
>+  /**
>+   * Sets the value of the cells in all columns in the current row to their
>+   * original value. Changes are not committed until the copyValues method is
>+   * called.
>+   */
>   void setDefaultValues();

the same as above

> 
>+  /**
>+   * Commits changes to the current row for all columns.
>+   */
>   void copyValues();
> 

needs to be fixed like copy()

> 
>+  /**
>+   * Inserts a row using the data assigned using the various setX methods.
>+   *
>+   * @return               1 if the row was inserted and 0 if the row was not.
>+   */
>   long insertRow();

actually, the return value means something different
-1 if there was an error, so the row was not inserted
0 the row was inserted but it doesn't fit the where condition, so it shouldn't
be displayed
1 the row was inserted and fits the condition

the same for updateRow()

deleteRow() returns -1 or 1

Comment 5

[Jan Varga [:janv]]

I plan to add/remove/rename some methods/attributes in future, but let's take
this comments first, they are very important

Comment 6

[Neil Deakin]

Attachment: Updated patch

Changes to mozISqlResultEnumerator.idl

Comment 7

[Jan Varga [:janv]]

Comment on attachment 132072 [details] [diff] [review]
Updated patch

r/moa=varga

Comment 8

[Owen Marshall (Not reading bugmail)]

Comment-only changes have a blanket SR, don't they?

Comment 9

[Jan Varga [:janv]]

Yeah, I think this can go in w/o an sr
Neil, you have a cvs login, don't you?

Comment 10

[Jan Varga [:janv]]

checked in, thanks Neil

Comment 11

[Owen Marshall (Not reading bugmail)]

Verified fixed.

Comment 12

[Owen Marshall (Not reading bugmail)]

*bugspam*