1161412 - Uplift to 39: Suggested Tiles pt1

Status: RESOLVED INVALID

(Firefox :: New Tab Page, defect)

Description

[Ed Lee :Mardak]

Attachment: combined 6 patches for aurora - Mardak will land as separate

First set of 6 uplifts of Suggested Tiles bug 1140185 patches that have baked on Nightly 40 for a while. No string changes at all with these. Tests updated except for the in-tree documentation patch.

This also includes the hardcoded buckets to help improve privacy aspects of suggested tiles.

Comment 1

[Liz Henry (:lizzard) (relman/hg->git project)]

What does "combined 6 patches for aurora - Mardak will land as separate" mean here? At first I thought you meant that this patch is from 6 other patches rolled into one. But you also call this part 1 of 6. 

Bug 1140185 doesn't have any patches directly on it. Which patches have landed? Is this from some of its dependent bugs?  Can you list which ones? Normally for uplift, we would check to see that code people want to uplift has landed on m-c.

Comment 2

[Ed Lee :Mardak]

(In reply to Liz Henry (:lizzard) from comment #1)
> At first I thought you meant that this patch is from 6 other patches rolled into one.
The attachment in this bug indeed has patches from the 6 depends on bugs: 1139052 1148862 1151876 1152145 1156876 1157503

> Bug 1140185 doesn't have any patches directly on it.
That bug is the parent tracking bug for the overall feature. I'm using these "Uplift to 39" bugs to track which functionality of that parent bug has been uplifted.

Comment 3

[Liz Henry (:lizzard) (relman/hg->git project)]

Ed, I would prefer that we land things like this from the individual bugs. That would help us to better track what functionality is getting uplifted. I still need to look at the patches in all the other bugs for context. 

Can you use the form that appears in the comments when you request uplift?  The format helps us to keep consistency and to parse the information more quickly since we look at a lot of bugs. 

Are there more bugs coming for uplift to 39?

Comment 4

[Ed Lee :Mardak]

(In reply to Liz Henry (:lizzard) from comment #3)
> Ed, I would prefer that we land things like this from the individual bugs.
In the past, we did this through combined patches for the whole functionality, because it didn't make sense to only uplift some of the bugs needed for the whole feature.

E.g., bug 1148859 comment 3 when we uplifted all of affiliate suggested tiles to 38.

This time I've split it into chunks of uplift (this is pt1, future pt2, etc.) because there was a desire to get wider-than-nightly testing on completed functionality sooner.

Do you want me to request uplift once for all suggested tiles functionality that needs to be part of the 39 release? Do it in smaller chunks? Make the request from each of the ~20 bugs?

> Can you use the form that appears in the comments when you request uplift?
Sorry, I tried to follow the form, but it didn't autopopulate when I created an attachment at the same time as creating the bug.

> Are there more bugs coming for uplift to 39?
Yes. I'm tracking those bugs to land on nightly and uplift to 39 in bug 1160372. I'm prioritizing the team to land strings in 40 then functionality that should bake longer then low risk changes.

Comment 5

[Liz Henry (:lizzard) (relman/hg->git project)]

I think from each of the bugs would it more clear what exactly is being uplifted. It leaves a clearer trail, and will help the uplift work to go faster.  

If you are filling in requests for a batch of a few bugs at once, you could fill out the form in one bug, and re-use it for the rest or just put in the bug numbers that will also need uplift and I can set the flags correctly in those bugs. It is not that I want to make you create the form for 20 bugs in a row; it's that it should be very clear in each bug that is uplifted that a patch was reviewed, landed in m-c, had some time there, and so on, for us to judge whether the uplift is ok to go or not. 

One other concern I have here is that, from the meeting and email thread, I thought we were only uplifting some minor UI and css changes for 39. Is that still more or less the case? Do you have a list of the bugs you intend to uplift that you could email me? Thanks!

Comment 6

[Liz Henry (:lizzard) (relman/hg->git project)]

Hmmm, I think I see what you mean -- the ones you intend to uplift are the ones blocking bug 1160372?

Comment 7

[Ed Lee :Mardak]

(In reply to Liz Henry (:lizzard) from comment #5)
> I think from each of the bugs would it more clear what exactly is being
> uplifted.
This bug is for uplifting 6 patches:

bug 1139052: Only show suggested tiles if the user has more than 8 top history sites
bug 1148862: Update pref to the new v3 endpoint
bug 1151876: A blocked link should not appear in the _topSitesWithSuggestedLinks cache or trigger
bug 1152145: Filter for specific suggested tiles adgroups/buckets/frecent_sites lists with display name
bug 1156876: Convert wiki page technical doc into in-tree .rst
bug 1157503: NewTabUtils.extractSite should try-catch nsIURI.asciiHost

As per comment 0, those have been in Nightly 40 for a while now, and some have been on Beta 38 (some changes were initially beta-only).

Approval Request Comment
[Feature/regressing bug #]: Suggested Tiles affiliate bug 1120311 + sponsored bug 1140185
[User impact if declined]: Suggested tiles are shown too often (whether of an unallowed bucket, or too soon without enough history, or triggered on a blocked tile).
[Describe test coverage new/current, TreeHerder]: There's added unit tests that have been running on Nightly 40 and Beta 38, and I've pushed these 6 patches together on aurora to try:  https://treeherder.mozilla.org/#/jobs?repo=try&revision=326f0d290ace
[Risks and why]: Low - code has been on nightly and beta for some time with a followup fixed in bug 1157503, and this code doesn't have downstream dependencies
[String/UUID change made/needed]: none

> One other concern I have here is that, from the meeting and email thread, I
> thought we were only uplifting some minor UI and css changes for 39.
lmandel's suggestion was to uplift the minor UI changes first before uplifting the complex logic changes. I have the team focused on string changes landing in Nightly 40 first to avoid breaking string freeze. The team is also focused on the complex changes so that they bake on m-c longer before I request uplift. I believe the goal is to reduce risk by having more testing.

(In reply to Liz Henry (:lizzard) from comment #6)
> Hmmm, I think I see what you mean -- the ones you intend to uplift are the
> ones blocking bug 1160372?
Those are additional bugs that are being implemented or are landing that I want to sit on nightly before requesting the next set of uplifts.

Comment 8

[Ed Lee :Mardak]

lizzard would like to have uplifts in each individual bug, so I'll get rid of these meta bugs tracking chunks of uplift.

Others can track what needs to be uplifted and what has been uplifted:

to uplift:
https://bugzilla.mozilla.org/buglist.cgi?f1=blocked&o1=equals&v1=1140185&f2=cf_status_firefox40&o2=anywords&v2=fixed+verified&o3=nowords&f3=cf_status_firefox39&v3=fixed+verified

uplifted:
https://bugzilla.mozilla.org/buglist.cgi?f1=blocked&o1=equals&v1=1140185&f2=cf_status_firefox40&o2=anywords&v2=fixed+verified&o3=anywords&f3=cf_status_firefox39&v3=fixed+verified