Closed
Bug 958119
Opened 10 years ago
Closed 10 years ago
Fixup style nits from Bug 939294
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: evilpie, Assigned: evilpie)
References
Details
Attachments
(1 file)
29.22 KB,
patch
|
gkrizsanits
:
review+
|
Details | Diff | Splinter Review |
I am going to fix those as soon as possible.
Comment 1•10 years ago
|
||
I talked to Bobby about this, and we agree that it's enough if you fix the isPrimitive/isObject part and just use whatever style the actual function signature uses (like you do). Also if you really don't have time for this for whatever reason just assign it to me, I don't want to hold you up with this too much.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → evilpies
Assignee | ||
Comment 2•10 years ago
|
||
So wait about that function signature stuff. From what I understand XPConnect at least right now, still uses JS-style, which is JSObject *foo.
Comment 3•10 years ago
|
||
(In reply to Tom Schuster [:evilpie] from comment #2) > So wait about that function signature stuff. From what I understand > XPConnect at least right now, still uses JS-style, which is JSObject *foo. Well... theoretically yes it should always use JS-style (so Class *foo etc.). But in reality a bunch of old files using all kind of styles sometimes even mixed in the same function, or even function signature. It would be great to clean up all this mess, little by little, but as Bobby pointed it out we should wait until the style related discussions settle somewhere, and there might be someone kind enough to write a script that fixes all this. It also probably gives little benefit if we fix the signatures like I suggested, and then in the function body, other style is used. So if it makes sense in the given context that you're editing to enforce JS-style, please do that. But it requires to much changes, or the whole file/function is using different style than just stick to that. If it's possible don't mix too many styles at least... It's quite a mess, so I don't think it makes sense for you to put too much effort into this for now.
Assignee | ||
Comment 4•10 years ago
|
||
I fixed most of the stuff that was obvious and some other random stuff. I didn't however feel like reformatting most of XPConnect in this patch ;)
Attachment #8361908 -
Flags: review?(gkrizsanits)
Comment 5•10 years ago
|
||
Comment on attachment 8361908 [details] [diff] [review] fix some Review of attachment 8361908 [details] [diff] [review]: ----------------------------------------------------------------- Holy moly Tom! (In reply to Tom Schuster [:evilpie] from comment #4) > I fixed most of the stuff that was obvious and some other random stuff. I > didn't however feel like reformatting most of XPConnect in this patch ;) Ah, yeah... only half of XPConnect I see ;) Thanks a lot for this, this is an awesome cleanup patch. I have a few nits, but even without those fixed I very r+ this patch. NS_IMETHODIMP mozJSComponentLoader::ImportInto(const nsACString & aLocation, JSObject *aTargetObj, - nsAXPCNativeCallContext * cc, + nsAXPCNativeCallContext *cc, JSObject **_retval) const nsACString &aLocation nsresult DoLoadSubScriptWithOptions(const nsAString& url, LoadSubScriptOptions& options, - JSContext* cx, + JSContext *cx, JS::MutableHandle<JS::Value> retval); & and * should follow the same rule.
Attachment #8361908 -
Flags: review?(gkrizsanits) → review+
Assignee | ||
Comment 6•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/888e98b2369a
Comment 7•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/888e98b2369a
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in
before you can comment on or make changes to this bug.
Description
•