Closed Bug 483739 Opened 11 years ago Closed 11 years ago

Establish style guidelines for storage

Categories

(Toolkit :: Storage, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: sdwilsh, Assigned: sdwilsh)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch v1.0 (obsolete) — Splinter Review
I want to get style guidelines established for the storage module so new contributors know exactly what to expect.

asuth - this review request is supposed to be more of a dialog since you are a peer.

Once we work these out, I'll start modifying our existing code to follow this style.
Attachment #367723 - Flags: review?(bugmail)
Whiteboard: [needs review asuth]
Patch is missing one rule so far that I've noticed:
* All functions that are not XPCOM should start with a lowercase letter
* Should local variables also be camelCased or is that left to the author's discretion?

* Should the license blocks have emacs and vim hints at the top?  A bunch (all?) of the files currently do, sometimes with the 'wrong' counts.  If so, the things to paste in should probably be in the style guide.

I always thought the 'a' was for argument?
(In reply to comment #2)
> * Should local variables also be camelCased or is that left to the author's
> discretion?
I think camelCase is our convention.  I'll add that.

> * Should the license blocks have emacs and vim hints at the top?  A bunch
> (all?) of the files currently do, sometimes with the 'wrong' counts.  If so,
> the things to paste in should probably be in the style guide.
Yup, need to add that too.

> I always thought the 'a' was for argument?
Well, it's an argument for the caller, but it's a parameter for the function.  You don't want to prefix it with p though, since that prefix is used for pointer.  It's a minor technicality.
Attachment #367723 - Flags: review?(bugmail) → review+
Comment on attachment 367723 [details] [diff] [review]
v1.0

fine with me with the promised additions
Whiteboard: [needs review asuth] → [has review]
Attached patch v1.1Splinter Review
Per comments.  We'll probably find more things to add here over time.
Attachment #367723 - Attachment is obsolete: true
Whiteboard: [has review] → [can land]
http://hg.mozilla.org/mozilla-central/rev/3407ca1084a6
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Flags: in-litmus-
Whiteboard: [can land]
Target Milestone: --- → mozilla1.9.2a1
You need to log in before you can comment on or make changes to this bug.