Closed
Bug 947736
Opened 7 years ago
Closed 7 years ago
Build modules/libpref/ in unified mode
Categories
(Core :: General, defect)
Core
General
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: ehsan, Assigned: ehsan)
References
Details
Attachments
(1 file, 1 obsolete file)
11.15 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8344374 -
Flags: review?(benjamin)
Comment 2•7 years ago
|
||
Comment on attachment 8344374 [details] [diff] [review] Build modules/libpref/ in unified mode What's up with nsPrefBranch::NotifyObserver returning an nsresult-as-int? That seems weird and there's no comment explaining it.
Attachment #8344374 -
Flags: review?(benjamin) → review-
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #2) > Comment on attachment 8344374 [details] [diff] [review] > Build modules/libpref/ in unified mode > > What's up with nsPrefBranch::NotifyObserver returning an nsresult-as-int? > That seems weird and there's no comment explaining it. I'm glad you asked. :-) We use that function as a callback argument. We define the type of that argument in two places, one with an nsresult return code, the other with an int return code: http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/prefapi.h#157 http://mxr.mozilla.org/mozilla-central/source/modules/libpref/public/Preferences.h#29 To make things more fun, as far as I can tell the return code is never used: <http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/Preferences.cpp#164>. But that doesn't really matter, since we always return NS_OK. So I just had to pick something and move on, hence the current patch. If you don't want to accept this patch then it's fine, but please suggest an alternative. Should I make this return void?
Flags: needinfo?(benjamin)
Comment 4•7 years ago
|
||
Yes, returning void seems better than living with conflicting typedefs.
Flags: needinfo?(benjamin)
Assignee | ||
Updated•7 years ago
|
Attachment #8344374 -
Attachment is obsolete: true
Assignee | ||
Comment 5•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8345413 -
Flags: review?(benjamin)
Updated•7 years ago
|
Attachment #8345413 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 6•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fe4ca46eec8
Assignee | ||
Comment 7•7 years ago
|
||
Backed out because of bustage: https://hg.mozilla.org/integration/mozilla-inbound/rev/982196bdc263
Assignee | ||
Comment 8•7 years ago
|
||
I had to edit all of the places which defined a pref callback function. https://hg.mozilla.org/integration/mozilla-inbound/rev/98be0e9cbc15
Comment 9•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/98be0e9cbc15
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in
before you can comment on or make changes to this bug.
Description
•