1715822 - Rename references to resultBuckets to resultGroups

Status: RESOLVED FIXED

(Firefox :: Address Bar, task, P1)

Description

[Harry Twyford]

Bug 1715484 renamed the pref browser.urlbar.resultBuckets to browser.urlbar.resultGroups. However, there are still references to result buckets throughout Firefox. Those should be turned into references to result groups. These SearchFox queries are good places to start looking.

https://searchfox.org/mozilla-central/search?q=resultBucket&path=

https://searchfox.org/mozilla-central/search?q=buckets&path=urlbar&case=false&regexp=false

Comment 1

[akritidm]

Hello,
May i work on this one as my first contribution?

Comment 2

[akritidm]

Attachment: diff of browser folder

Comment 3

[Harry Twyford]

Thank you for volunteering! Could you please post your patch on Phabricator? You can follow the instructions in this guide to do that. You also missed references to result buckets. Check the queries I posted in comment 0 for a reference to all the places you need to look, like test files and toolkit/components/places/UnifiedComplete.jsm. Do not modify the files in the security folder, as those are unrelated.

Comment 4

[akritidm]

Should i change all "bucket(s)" references in the urlbar directory? I decided to change only the resultBucket(s) references as to not mess anything up.

Comment 5

[akritidm]

Oh ok, now i see my attached diff does not show all of my changes. I'm working on posting with Phabricator.

Comment 6

[Harry Twyford]

(In reply to akritidm from comment #4)

Should i change all "bucket(s)" references in the urlbar directory? I decided to change only the resultBucket(s) references as to not mess anything up.

Yes, all references should be updated as far as I know. If I notice any that shouldn't be, I'll point them out in review.

Comment 7

[akritidm]

Attachment: Bug 1715822 - Replaced references to result buckets with result groups. r?harry

Comment 8

[akritidm]

Sorry for asking here, i didnt know how to contact you. How and when can i remove the telemetry.rst.orig?

Comment 9

[Harry Twyford]

(In reply to akritidm from comment #8)

Sorry for asking here, i didnt know how to contact you. How and when can i remove the telemetry.rst.orig?

hg remove browser/components/urlbar/docs/telemetry.rst.orig should do it.

Comment 10

[akritidm]

I did the hg remove, then hg commit and moz-phab. I have a previous commit pilled up with this one. This is the output im getting:

Starting up..
Checking connection to Phabricator.
Looking for commits..
Loading commits..
Submitting 3 commits as Work In Progress
Loading existing revisions...
Loading diffs...
(    New) 583824:3456ca351c5d Removed telemetry.rst.orig
!! Missing Bug ID
!! Missing reviewers
   It will be submitted as "Changes Planned".
   Run submit again with --no-wip to prevent this.
(    New) 583823:155ada26b0fd Removed telemetry.rst.orig
!! Missing Bug ID
!! Missing reviewers
   It will be submitted as "Changes Planned".
   Run submit again with --no-wip to prevent this.
Figuring out who you are...
(D118406) 583822:78bee6008bf0 Bug 1715822
 * This revision has not changed and will not be submitted.
Checking commits..
Unable to submit commits:

583823:155ada26b0fd Removed telemetry.rst.orig
- missing bug-id

583824:3456ca351c5d Removed telemetry.rst.orig
- missing bug-id

Would appreciate some help at this point.

Comment 11

[Harry Twyford]

When writing commit messages for Firefox bugs, they should take the format

Bug <Bug number> - <Commit message>. r?<reviewer name>

So for this bug, it would be

Bug 1715822 - Replaced references to result buckets with result groups. r?harry

Renaming your commit should resolve the issues with moz-phab. Also please squash all commits together. You can do this with hg histedit. You don't need a separate commit for removing telemetry.rst.orig.

Comment 12

[Harry Twyford]

akritidm, are you still interested in working on this? Your patch was nearly complete.

Comment 13

[akritidm]

I intend on finishing this. Been caught up with some other stuff lately and couldnt find the time to complete it.

Comment 14

[Harry Twyford]

Could you please finish this patch by the end of the week? Otherwise, I'll take it over to finish up the last touches and land it.

Comment 15

[Harry Twyford]

Clearing assignee so someone else can pick this up. akritidm, you're still free to pick this back up if you wish.

Comment 16

[Leslie]

Hi Harry, my name is Leslie and I am an Outreachy applicant and I'd like to work on this bug. Can I be assigned to it?

Comment 17

[Makenzie]

Hello, I'm an Outreachy Applicant, can I just continue on with the work Harry was doing and submit a push request? Or do I need to be assigned first?

Comment 18

[Makenzie]

(In reply to Makenzie from comment #17)

Hello, I'm an Outreachy Applicant, can I just continue on with the work Harry was doing and submit a push request? Or do I need to be assigned first?

Sorry I just realized that Harry, you're the reporter, woops. Anyways, this looks like a great first bug for me and I'd love to contribute.

Comment 19

[Harry Twyford]

Thanks for all the interest! Makenzie, I'll let Leslie make a first attempt at this bug since she asked first. Leslie, the first step to fixing this bug is setting up your Firefox dev environment. Once you're set up, install the MozPhab command line tool:

./mach install-moz-phab

You can install the unfinished patch locally with moz-phab patch D118406 --apply-to central. That will apply the patch to the central branch of Firefox. Once you're done with the patch, you can push it for review just by running moz-phab. It will probably give you a message about how the revision already belongs to someone else. You can just confirm that you want to take it over. If it doesn't let you, try going to the patch's Phabricator page, scrolling to the bottom and selecting "Commandeer Revision". Then try again from the command line. If that still doesn't work, please needinfo me again.

Comment 20

[Leslie]

Attachment: Bug 1715822 - Rename references from resultBuckets to resultGroups. r?harry

Depends on D128038

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8652d1a2a2fa
Rename references from resultBuckets to resultGroups. r=harry

Comment 22

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/8652d1a2a2fa

Comment 23

[Drew Willcoxon :adw]

[Tracking Requested - why for this release]: We need this for the Firefox Suggest preferences redesign targeting 95/94.

Comment 24

[Drew Willcoxon :adw]

Comment on attachment 9245674 [details]
Bug 1715822 - Rename references from resultBuckets to resultGroups. r?harry

Beta/Release Uplift Approval Request

• User impact if declined: We need this for the Firefox Suggest preferences redesign targeting 95/94.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: Please see uplift spreadsheet: https://docs.google.com/spreadsheets/d/1LavihS-VOPFYEyum7mrx6FKXmuQeHi9xQHfGNSxjnoY/edit?usp=sharing
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This revision only changes the names of some variables and other identifiers in the code. There are no user-facing changes. All this code is covered by automated tests.
• String changes made/needed:

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9245674 [details]
Bug 1715822 - Rename references from resultBuckets to resultGroups. r?harry

Approved for 94.0.2.

Comment 26

[BugBot [:suhaib / :marco/ :calixte]]

Changing the priority to p1 as the bug is tracked by a release manager for the current release.
See What Do You Triage for more information

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/c880b2165090