Closed
Bug 621103
Opened 15 years ago
Closed 15 years ago
Should js::Anchor be JS::Anchor?
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | .x+ |
People
(Reporter: jimb, Assigned: jimb)
Details
(Whiteboard: [fixed-in-tracemonkey])
Attachments
(1 file)
5.25 KB,
patch
|
Waldo
:
review+
dmandelin
:
approval2.0+
|
Details | Diff | Splinter Review |
Waldo tells me public C++ API should be the JS:: namespace, and the js:: namespace is reserved for internal stuff. If that's true, then js::Anchor should be JS::Anchor.
Is this, in fact, the case?
![]() |
||
Comment 1•15 years ago
|
||
I don't know.
Comment 2•15 years ago
|
||
$ grep '^namespace ' jsapi.h
namespace js {
namespace JS {
It's early but the first one was JS. It fits the old API style, However, that first occurrence was just C-like API with :: instead of _ and I objected that we should stick with extern "C" in jsapi.h where there's no C++ win.
Anchors are C++ win, of course. Are we ready to put them out under JS::? I guess.
/be
Assignee | ||
Comment 3•15 years ago
|
||
Assignee: general → jimb
Attachment #507925 -
Flags: review?
Assignee | ||
Updated•15 years ago
|
Attachment #507925 -
Flags: review? → review?(jwalden+bmo)
Comment 4•15 years ago
|
||
Comment on attachment 507925 [details] [diff] [review]
Rename js::Anchor to JS::Anchor: 'JS' is the public namespace.
rs=me
Attachment #507925 -
Flags: review?(jwalden+bmo) → review+
Assignee | ||
Comment 5•15 years ago
|
||
Okay. Unless someone can argue that this should block, I'll land this after FF4, I guess.
blocking2.0: --- → ?
Comment 6•15 years ago
|
||
This seems like a "why bother waiting?" fix to me that you should just land.
Comment 7•15 years ago
|
||
(In reply to comment #6)
> This seems like a "why bother waiting?" fix to me that you should just land.
Agreed.
blocking2.0: ? → .x
Updated•15 years ago
|
Attachment #507925 -
Flags: approval2.0+
Assignee | ||
Comment 8•15 years ago
|
||
Assignee | ||
Updated•15 years ago
|
Status: NEW → ASSIGNED
Whiteboard: [fixed-in-tracemonkey]
Comment 9•15 years ago
|
||
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/8eb17a6a39aa
Updated•15 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•