1061804 - Browser migration operator variant followup

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Browser, defect)

Description

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Due to the fact bug 940647 already went into master, filing a followup to fix the extra requests from bug 940647 here, since we'll need uplift flags now.

Comment 1

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

[Blocking Requested - why for this release]: Fixes/stablization for bookmark migration.

Comment 2

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Fixes required are in comments for: https://github.com/mozilla-b2g/gaia/pull/23400

Comment 3

[Carmen Jimenez Cabezas]

The landed patch in bug 940647 doesn't have the changes needed to process the single variant configuration for bookmarks during the build process. Since I couldn't find the bug to generate that configuration, I've opened bug 1061750 for that. Please feel free to close it as duplicated if the bug is already opened!

Without that change, this code is not going to be executed ever.

Please note that the proposed patch for bug 1061750 assumes that on this bug at least the change in [1]
is implemented here.

[1] https://github.com/mozilla-b2g/gaia/pull/23400#discussion-diff-16965552R15

Comment 4

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v1) - Fix Browser Customization format and tests

Here's a patch with the fixes recommended in Bug 940647. I ended up using sinon spy on BookmarksDatabase.add, since that's where we're worried about things ending up anyways.

Comment 5

[Carmen Jimenez Cabezas]

Comment on attachment 8483191 [details] [review]
Patch 1 (v1) - Fix Browser Customization format and tests

Thanks a lot for the changes. I've left some comments on github. Please request the review again after you've checked them!

Comment 6

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Made all requested changes and repushed.

Comment 7

[Carmen Jimenez Cabezas]

Comment on attachment 8483191 [details] [review]
Patch 1 (v1) - Fix Browser Customization format and tests

Nice, thank you Kyle!

Comment 8

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8483191 [details] [review]
Patch 1 (v1) - Fix Browser Customization format and tests

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Bug 940647
[User impact] if declined: Bookmark operator variant won't work
[Testing completed]: Unit tests in gaia
[Risk to taking this patch] (and alternatives if risky): None known
[String changes made]: None

Comment 9

[bhavana bajaj [:bajaj]]

waiting for the master commit and for this to land on 2.2 before approving on the branch. Please seek approval on a branch, once you bug is fixed on master/central.

Comment 10

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

It did land, I forgot to paste in the commit.

https://github.com/mozilla-b2g/gaia/commit/21855808580585fe96c53930700c5991c5b51c93

Comment 11

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/4ebc5b7bfa4e85a25160d54197114ef5df61aa8f

Comment 12

[Kevin Grandon :kgrandon]

in-testsuite- for bugs which can not yet be tested with a marionette test.