Closed Bug 487145 Opened 15 years ago Closed 15 years ago

Update language helpers to follow style guidelines

Categories

(Toolkit :: Storage, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: sdwilsh, Assigned: sdwilsh)

Details

Attachments

(1 file)

      No description provided.
Attached patch v1.0Splinter Review
Sorry this is such a big patch.  -w doesn't help it seems either...

No functional changes.
Attachment #371514 - Flags: review?(bugmail)
Comment on attachment 371514 [details] [diff] [review]
v1.0

I'm fine with raw patches from your queue, and it's probably preferable when whitespace is involved.  I pulled 	storage.update-statement-lang-helpers-style from http://hg.mozilla.org/users/sdwilsh_shawnwilsher.com/storage-async/ to try and have that, but it lacks the mozStorageStatementRow.* changes so I suspect that is not completely up-to-date.  I think the patch provided attached is fine, though.

xpc_map_end.h is awesome.  To show what a good reviewer I am, I will note that following code in mozStorageStatementParams::GetClassName has no equivalent in xpc_end_map, but since only XPConnect should call the function and it does not violate such constraints, it should be fine:
  NS_ENSURE_ARG_POINTER(aClassName);
  if (!*aClassName)
    return NS_ERROR_OUT_OF_MEMORY;

nits:

in StatementParams::NewResolve, there are some indentation glitches:
+    // Ensure that our index is within range.
+    if (idx >= mParamCount)
+        return NS_ERROR_INVALID_ARG;

+    nsresult rv = mStatement->GetParameterIndex(name, &idx);
+    if (NS_FAILED(rv))
+        return NS_OK;


in StatementRow::GetProperty, is the second line of the supposed to just be 2-space indented?  I find it awkward when the clause looks like it lines up with the code that immediately follows it (especially with same-line braces):
    if (type == mozIStorageValueArray::VALUE_TYPE_INTEGER ||
      type == mozIStorageValueArray::VALUE_TYPE_FLOAT) {
      double dval;
Attachment #371514 - Flags: review?(bugmail) → review+
(In reply to comment #2)
> in StatementRow::GetProperty, is the second line of the supposed to just be
> 2-space indented?  I find it awkward when the clause looks like it lines up
> with the code that immediately follows it (especially with same-line braces):
>     if (type == mozIStorageValueArray::VALUE_TYPE_INTEGER ||
>       type == mozIStorageValueArray::VALUE_TYPE_FLOAT) {
>       double dval;
I missed that because this file was 4-space indented, so the code and if block were lined up!
http://hg.mozilla.org/mozilla-central/rev/a41ec1a6b8fc
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Flags: in-testsuite-
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: