Closed Bug 1351067 Opened 8 years ago Closed 8 years ago

add BUG_COMPONENT to toolkit/* files

Categories

(Toolkit :: General, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jmaher, Assigned: jmaher)

References

Details

Attachments

(1 file, 2 obsolete files)

to help associate source/test files with proper bugzilla components.
Attached patch add BUG_COMPONENT to toolkit/* (obsolete) — Splinter Review
this is derived from looking at the bugs where these files were introduced and what components they are in- I am happy with a r- and some general direction. A lot of the "Toolkit::XUL Widgets" are due to moving old files, likewise the embedded APIs is all when we moved a bunch of files a few months ago and there is no history in the easy hg view. Happy to get more precise or less precise :)
Assignee: nobody → jmaher
Status: NEW → ASSIGNED
Attachment #8851757 - Flags: review?(myk)
Attachment #8851757 - Flags: review?(enndeakin)
I don't know what many of the toolkit/components subdirectories are so I can't help much there. The tests probably don't matter too much. Some things I did note: diff --git a/toolkit/components/filepicker/moz.build b/toolkit/components/filepicker/moz.build +with Files('**'): + BUG_COMPONENT = ('Toolkit', 'Autocomplete') + This seems like it should be something else. This code is for the gtk-only filepicker. diff --git a/toolkit/components/printingui/moz.build b/toolkit/components/printingui/moz.build +with Files('**'): + BUG_COMPONENT = ('Core', 'Embedding: APIs') + Should be Toolkit/Printing diff --git a/toolkit/components/urlformatter/moz.build b/toolkit/components/urlformatter/moz.build +with Files('**'): + BUG_COMPONENT = ('Toolkit', 'Add-ons Manager') + This seems like it should be something else. diff --git a/toolkit/mozapps/preferences/moz.build b/toolkit/mozapps/preferences/moz.build +with Files('**'): + BUG_COMPONENT = ('Toolkit', 'XUL Widgets') + Toolkit/Preferences I would think. diff --git a/toolkit/xre/moz.build b/toolkit/xre/moz.build +with Files('**'): + BUG_COMPONENT = ('Toolkit', 'General') + This should probably be Startup and Profile System
Attachment #8851757 - Flags: review?(enndeakin)
thanks, I will update this!
Comment on attachment 8851757 [details] [diff] [review] add BUG_COMPONENT to toolkit/* Review of attachment 8851757 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/browser/moz.build @@ +4,5 @@ > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +with Files('**'): > + BUG_COMPONENT = ('Core', 'Embedding: APIs') None of the code that was formerly in embedding/ should be tracked by Core :: Embedding: APIs, as we moved the code out of that directory specifically because we're no longer maintaining it as embedding APIs (but rather only using it internally for Firefox and other Toolkit-based apps). These directories should be tracked by the Toolkit product. This one in particular should probably be in the General component, as I don't know of a more specific component for it. ::: toolkit/components/printingui/moz.build @@ +4,5 @@ > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +with Files('**'): > + BUG_COMPONENT = ('Core', 'Embedding: APIs') This one should be in Core :: Printing, whose description is "Print Preview and other toolkit elements of printing." ::: toolkit/components/windowcreator/moz.build @@ +4,5 @@ > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +with Files('**'): > + BUG_COMPONENT = ('Toolkit', 'Embedding: APIs') This one should be Toolkit :: General, as there doesn't seem to be a more specific component. ::: toolkit/components/windowwatcher/moz.build @@ +4,5 @@ > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +with Files('**'): > + BUG_COMPONENT = ('Toolkit', 'Embedding: APIs') This one should be Toolkit :: General as well.
Attachment #8851757 - Flags: review?(myk) → review-
Attached patch add BUG_COMPONENT to toolkit/* (obsolete) — Splinter Review
I have addressed the previous feedback! Please feel free to redirect the review or recommend other reviewers.
Attachment #8851757 - Attachment is obsolete: true
Attachment #8854862 - Flags: review?(myk)
Comment on attachment 8854862 [details] [diff] [review] add BUG_COMPONENT to toolkit/* Review of attachment 8854862 [details] [diff] [review]: ----------------------------------------------------------------- I only took a second look at the four designations that were previously incorrect, and their new values look good. But I'm not a Toolkit/Firefox owner/peer, and one of those should grant a final r+. I hesitate to suggest mossop (the owner of both modules), since he's probably pretty busy, but since these are ontological questions, it's worth getting his perspective.
Attachment #8854862 - Flags: review?(myk)
Attachment #8854862 - Flags: review?(dtownsend)
Attachment #8854862 - Flags: feedback+
Comment on attachment 8854862 [details] [diff] [review] add BUG_COMPONENT to toolkit/* Review of attachment 8854862 [details] [diff] [review]: ----------------------------------------------------------------- I'd like to skim over this again after these amendments ::: toolkit/components/addoncompat/moz.build @@ +2,5 @@ > # vim: set filetype=python: > # This Source Code Form is subject to the terms of the Mozilla Public > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > + Nit: whitespace ::: toolkit/components/exthelper/moz.build @@ +4,5 @@ > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +with Files('**'): > + BUG_COMPONENT = ('Firefox', 'General') Toolkit::General Though hopefully we can remove this after 57. ::: toolkit/components/feeds/moz.build @@ +4,5 @@ > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +with Files('**'): > + BUG_COMPONENT = ('Toolkit', 'General') Firefox::RSS Discovery and Preview ::: toolkit/components/mozprotocol/moz.build @@ +4,5 @@ > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +with Files('**'): > + BUG_COMPONENT = ('Firefox', 'General') Toolkit::General ::: toolkit/components/processsingleton/moz.build @@ +4,5 @@ > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +with Files('**'): > + BUG_COMPONENT = ('Firefox', 'Developer Tools') Toolkit::General ::: toolkit/components/reflect/moz.build @@ +4,5 @@ > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +with Files('**'): > + BUG_COMPONENT = ('Toolkit', 'General') Core::JavaScript Engine ::: toolkit/components/remote/moz.build @@ +4,5 @@ > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +with Files('**'): > + BUG_COMPONENT = ('Toolkit', 'General') Toolkit::Startup and Profile System ::: toolkit/components/remotebrowserutils/moz.build @@ +4,5 @@ > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +with Files('**'): > + BUG_COMPONENT = ('Firefox', 'Session Restore') Core::Document Navigation ::: toolkit/components/social/test/xpcshell/.eslintrc.js @@ -3,5 @@ > -module.exports = { > - "extends": [ > - "plugin:mozilla/xpcshell-test" > - ] > -}; Why is this being removed in this patch? ::: toolkit/components/utils/moz.build @@ +4,5 @@ > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > +with Files('**'): > + BUG_COMPONENT = ('Core', 'Networking') Toolkit::General ::: toolkit/content/moz.build @@ +206,4 @@ > with Files('widgets/*'): > BUG_COMPONENT = ('Toolkit', 'XUL Widgets') > > +with Files('**'): Does ordering matter here? I'd expect this to need to be either first or last if so. @@ +221,5 @@ > +with Files('aboutProfile*'): > + BUG_COMPONENT = ('Toolkit', 'Startup and Profile System') > + > +with Files('aboutRights*'): > + BUG_COMPONENT = ('Toolkit Graveyard', 'XULRunner') Toolkit::General @@ +242,5 @@ > +with Files('buildconfig.html'): > + BUG_COMPONENT = ('Toolkit', 'Build Config') > + > +with Files('contentAreaUtils.js'): > + BUG_COMPONENT = ('Toolkit Graveyard', 'XULRunner') Toolkit::General @@ +266,5 @@ > +with Files('menulist.css'): > + BUG_COMPONENT = ('Toolkit', 'XUL Widgets') > + > +with Files('minimal-xul.css'): > + BUG_COMPONENT = ('Core', 'CSS Parsing and Computation') Toolkit::XUL Widgets @@ +281,5 @@ > +with Files('timePicker*'): > + BUG_COMPONENT = ('Toolkit', 'XUL Widgets') > + > +with Files('treeUtils.js'): > + BUG_COMPONENT = ('Firefox', 'Page Info Window') Toolkit::General @@ +284,5 @@ > +with Files('treeUtils.js'): > + BUG_COMPONENT = ('Firefox', 'Page Info Window') > + > +with Files('viewZoomOverlay.js'): > + BUG_COMPONENT = ('Toolkit', 'View Source') Toolkit::General ::: toolkit/modules/moz.build @@ +151,5 @@ > +with Files('Sqlite.jsm'): > + BUG_COMPONENT = ('Toolkit', 'Storage') > + > +with Files('UpdateUtils.jsm'): > + BUG_COMPONENT = ('Toolkit', 'Add-ons Manager') Toolkit::General @@ +154,5 @@ > +with Files('UpdateUtils.jsm'): > + BUG_COMPONENT = ('Toolkit', 'Add-ons Manager') > + > +with Files('WindowsRegistry.jsm'): > + BUG_COMPONENT = ('Firefox', 'Theme') Toolkit::General
Attachment #8854862 - Flags: review?(dtownsend)
updated to address all the comments. One question about deleting the .eslintjs file- this is the only file in the directory, there are no other files, so instead of assigning this to toolkit general, I removed to single eslint file and it removed the directory reference as well. I am happy to leave it in though.
Attachment #8854862 - Attachment is obsolete: true
Attachment #8855481 - Flags: review?(dtownsend)
Attachment #8855481 - Flags: review?(dtownsend) → review+
Pushed by jmaher@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/03913270e54c add BUG_COMPONENT to toolkit/* files. r=myk,enndeakin,mossop
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1355089
No longer depends on: 1355089
See Also: → 1594859
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: