Closed
Bug 1333493
Opened 7 years ago
Closed 7 years ago
add BUG_COMPONENT to js/* files
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: jmaher, Assigned: jmaher)
References
Details
Attachments
(2 files, 2 obsolete files)
1.68 KB,
patch
|
Details | Diff | Splinter Review | |
3.45 KB,
patch
|
bbouvier
:
review+
|
Details | Diff | Splinter Review |
In order to file bugs in the right locations and to establish a point of contact for source/test files, it is important to know what bugzilla component each file should be associated with. I looked through some of the files and tried to figure it out, here is what I have: Core::JavaScript Engine js/ductwork/* js/examples/* js/public/* Core::JavaScript Engine, or JavaScript Engine: GC ? js/ipc/* Core::XPConnect? js/xpconnect/* js/src/ has a lot of subdirs, I suspect those could be split up more accordingly. I sort of gave up here. Here are some bugzilla components which I believe might be interesting to ensure we have source/test files mapped to: Core::JavaScript Engine Core::JavaScript Engine: GC Core::JavaScript Engine: JIT Core::JavaScript Engine: Standard Library Core::JavaScript Engine: Internationalization API Core::js-ctypes
Assignee | ||
Comment 1•7 years ago
|
||
:jorendorff, can you help me out and recommend bugzilla components for either specific files, file patterns, or sub directories?
Flags: needinfo?(jorendorff)
Comment 2•7 years ago
|
||
For what it's worth, /js/src/moz.build already contains a few mappings we made in the past.
Assignee | ||
Comment 3•7 years ago
|
||
excellent, possibly we can quickly validate those and ensure the other files under js/* have a reasonable component.
Assignee | ||
Comment 4•7 years ago
|
||
proposed patch in case the answer is simple :)
Assignee: nobody → jmaher
Status: NEW → ASSIGNED
Comment 5•7 years ago
|
||
In addition to what's in this patch: js/src/jit/** => JavaScript Engine: JIT js/src/gc/** => JavaScript: GC js/src/builtin/Intl* => JavaScript: Internationalization API js/src/builtin/** => JavaScript: Standard Library
Flags: needinfo?(jorendorff)
Assignee | ||
Comment 6•7 years ago
|
||
oh, in fact those are already in place for js/src: https://dxr.mozilla.org/mozilla-central/source/js/src/moz.build that is cool! What is missing is some of the top level files and the public/ directory- I am happy to put everything else as Core::JavaScript Engine if that makes sense.
Comment 7•7 years ago
|
||
(In reply to Joel Maher ( :jmaher) from comment #6) > What is missing is some of the top level files and the public/ directory- I > am happy to put everything else as Core::JavaScript Engine if that makes > sense. Sounds good. Thanks.
Assignee | ||
Comment 8•7 years ago
|
||
thanks for the feedback, this patch should do the trick!
Attachment #8831161 -
Attachment is obsolete: true
Attachment #8833284 -
Flags: review?(jorendorff)
Assignee | ||
Comment 9•7 years ago
|
||
checking in here- it has been 2 weeks...feel free to redirect the review as needed.
Assignee | ||
Comment 10•7 years ago
|
||
:naveed- can you help me find someone to review this patch and the patch in bug 1344878?
Flags: needinfo?(nihsanullah)
Comment 11•7 years ago
|
||
I did the initial work for js/src, I'll steal the review to help things move forward.
Flags: needinfo?(nihsanullah)
Comment 12•7 years ago
|
||
Comment on attachment 8833284 [details] [diff] [review] add BUG_COMPONENT to js/* Review of attachment 8833284 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, thanks. ::: js/ductwork/debugger/moz.build @@ +22,5 @@ > > FINAL_LIBRARY = 'xul' > + > +with Files("**"): > + BUG_COMPONENT = ("Core", "JavaScript Engine") I've got a nice preference for keeping this at the top of the file, like for other files (js/src/moz.build in particular). ::: js/moz.build @@ +1,2 @@ > +with Files("**"): > + BUG_COMPONENT = ("Core", "JavaScript Engine") For the case of js/public, which doesn't have a moz.build file, we decided to use a relative reference in js/src/moz.build: `with Files("../public"):` Would it make sense to do the same here, to prevent scattering?
Attachment #8833284 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 13•7 years ago
|
||
thanks for the notes, I have moved the work to the top of the file, but on the second point, I see some issues: it appears the ../ notation doesn't work for BUG_COMPONENTS: $ ./mach file-info bugzilla-component js/public/** UNKNOWN js/public/CallArgs.h js/public/CallNonGenericMethod.h js/public/CharacterEncoding.h js/public/Class.h js/public/Conversions.h js/public/Date.h js/public/Debug.h js/public/GCAPI.h js/public/GCAnnotations.h js/public/GCHashTable.h js/public/GCPolicyAPI.h js/public/GCVariant.h js/public/GCVector.h js/public/HashTable.h js/public/HeapAPI.h js/public/Id.h js/public/Initialization.h js/public/LegacyIntTypes.h js/public/MemoryMetrics.h js/public/Principals.h js/public/ProfilingFrameIterator.h js/public/ProfilingStack.h js/public/Proxy.h js/public/Realm.h js/public/RefCounted.h js/public/RequiredDefines.h js/public/Result.h js/public/RootingAPI.h js/public/SliceBudget.h js/public/StructuredClone.h js/public/SweepingAPI.h js/public/TraceKind.h js/public/TracingAPI.h js/public/TrackedOptimizationInfo.h js/public/TypeDecls.h js/public/UbiNode.h js/public/UbiNodeBreadthFirst.h js/public/UbiNodeCensus.h js/public/UbiNodeDominatorTree.h js/public/UbiNodePostOrder.h js/public/UbiNodeShortestPaths.h js/public/UniquePtr.h js/public/Utility.h js/public/Value.h js/public/Vector.h js/public/WeakMapPtr.h I took a stab at moving all of existing ../public references to the new root level js/moz.build file. I know you wanted to avoid that which I tried to do until I realized it wasn't working for ../public. If you are not comfortable with this, possibly we can look at working on the moz.build code for handing BUG_COMPONENTS.
Attachment #8833284 -
Attachment is obsolete: true
Attachment #8850464 -
Flags: review?(bbouvier)
Assignee | ||
Comment 14•7 years ago
|
||
forgot to pick up recent changes, this looks right
Attachment #8850464 -
Attachment is obsolete: true
Attachment #8850464 -
Flags: review?(bbouvier)
Attachment #8850470 -
Flags: review?(bbouvier)
Comment 15•7 years ago
|
||
Comment on attachment 8850464 [details] [diff] [review] add BUG_COMPONENT to js/* Review of attachment 8850464 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine to me; would the lines for public/ files from js/src/moz.build need to be removed, then? (rs=me for that) Thanks! ::: js/moz.build @@ +4,5 @@ > + > +with Files("**"): > + BUG_COMPONENT = component_engine > + > +with Files("examples/**"): Out of curiosity, doesn't the line `with Files("**")` already cover the examples, ipc and public directories? (as well as ductwork/debugger) @@ +13,5 @@ > + > +with Files('public/**'): > + BUG_COMPONENT = component_engine > + > +for header in ('GCAnnotations.h', 'GCAPI.h', 'HeapAPI.h', 'RootingAPI.h', 'SliceBudget.h', 'SweepingAPI.h', 'TraceKind.h', 'TracingAPI.h', 'WeakMapPtr.h'): Can you please add here: GCHashTable.h, GCPolicyAPI.h, GCVariant.h, GCVector.h? @@ +20,5 @@ > + > +with Files('public/TrackedOptimizationInfo.h'): > + BUG_COMPONENT = component_jit > + > + nit: 2 unnecessary blank lines at EOF
Attachment #8850464 -
Attachment is obsolete: false
Comment 16•7 years ago
|
||
Comment on attachment 8850470 [details] [diff] [review] add BUG_COMPONENT to js/* Review of attachment 8850470 [details] [diff] [review]: ----------------------------------------------------------------- oops, mid-air collision; see above comments, some of which still apply here. Thanks!
Attachment #8850470 -
Flags: review?(bbouvier) → review+
Comment 17•7 years ago
|
||
Pushed by jmaher@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/91d1dce57468 add BUG_COMPONENT to js/* files. r=bbouvier
Comment 18•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/91d1dce57468
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•