Closed Bug 1351067 Opened 2 years ago Closed 2 years ago

add BUG_COMPONENT to toolkit/* files

Categories

(Toolkit :: General, enhancement)

enhancement
Not set

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
https://hg.mozilla.org/mozilla-central/rev/03913270e54c
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1355089
No longer depends on: 1355089
You need to log in before you can comment on or make changes to this bug.