Closed
Bug 1351067
Opened 8 years ago
Closed 8 years ago
add BUG_COMPONENT to toolkit/* files
Categories
(Toolkit :: General, enhancement)
Toolkit
General
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: jmaher, Assigned: jmaher)
References
Details
Attachments
(1 file, 2 obsolete files)
53.76 KB,
patch
|
mossop
:
review+
|
Details | Diff | Splinter Review |
to help associate source/test files with proper bugzilla components.
Assignee | ||
Comment 1•8 years ago
|
||
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)
Comment 2•8 years ago
|
||
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
Updated•8 years ago
|
Attachment #8851757 -
Flags: review?(enndeakin)
Assignee | ||
Comment 3•8 years ago
|
||
thanks, I will update this!
Comment 4•8 years ago
|
||
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-
Assignee | ||
Comment 5•8 years ago
|
||
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 6•8 years ago
|
||
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 7•8 years ago
|
||
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)
Assignee | ||
Comment 8•8 years ago
|
||
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)
Updated•8 years ago
|
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
![]() |
||
Comment 10•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 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
•