957169 - Partner repack changes for web.de, gmx, and mail.com (Firefox 27)

Status: RESOLVED INCOMPLETE

(Release Engineering :: Release Requests, defect)

Description

[Mike Kaply [:mkaply]]

Attachment: ff27.diff

Diff attached.

I will check this in once the builds have been approved by our QA.

Comment 1

[Mike Kaply [:mkaply]]

Comment on attachment 8356610 [details] [diff] [review]
ff27.diff

Review of attachment 8356610 [details] [diff] [review]:
-----------------------------------------------------------------

Ben Hearsum:

Do these patches need to be reviewed every time?

Or since they are our builds, can I just check in.

Comment 2

[[e-mail] (:bhearsum)]

(In reply to Mike Kaply (:mkaply) from comment #1)
> Comment on attachment 8356610 [details] [diff] [review]
> ff27.diff
> 
> Review of attachment 8356610 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Ben Hearsum:
> 
> Do these patches need to be reviewed every time?
> 
> Or since they are our builds, can I just check in.

I'm going to defer to Mike on this...

Comment 3

[Rail Aliiev [:rail]]

Assingning to :mkaply since he has a patch to land.

Comment 4

[Chris Cooper [:coop] (he/him)]

mkaply: has QA validated your builds, i.e. are you ready to land? 

The window to get this patch in before Firefox 27 release automation starts is closing.

Comment 5

[Mike Kaply [:mkaply]]

I received no info from QA, so I'll take that as a good thing.

I went ahead and pushed the changes.

Comment 6

[Mike Kaply [:mkaply]]

I didn't mean to clear the needinfo for mconnor

Comment 7

[Mike Connor [:mconnor]]

Huh, I could have sworn I commented on this weeks ago.

In any case, here's the guidelines we should apply:

If the changes are entirely of the following nature, I'm fine with a blanket a= based on a good faith understanding with mkaply:

* Updates to bundled versions of add-ons that have already been approved by AMO
* Version/compatibility bumps
* String/cosmetic changes with no functional impact

If there are functional changes to add-on code that hit any/all of the below criteria, we need to review the changes:

* Add-on updates that have not been submitted and approved to AMO
* Any changes to core prefs (i.e. changed prefs in distribution.ini)
* Any changes to /searchplugins
* Any other change that potentially impacts the privacy or security of Firefox users from the default.

If the changes are a mix of the two lists, ideally we'd split out changes along those lines to make for easier/quicker review turnarounds.

In this case, a quick skim indicates there are changes to the tracking code to include additional data, so that alone should be explained and understood before we approve anything.  If it's been signed off by AMO I'll trust their judgement.

Comment 8

[Mike Kaply [:mkaply]]

These particular changes haven't made it to AMO.

As far as the tracking goes, we updated the tracking (which was already approved), to include the browser version and whether or not our toolbar was in its normal state or had been minimized to just a button.