59 bytes, text/x-review-board-request
`MOBILE_BATCH_SIZE`, `DEFAULT_MOBILE_GUID_FETCH_BATCH_SIZE`, and the `client.type` pref seem to be leftovers, maybe from XUL Fennec? Let's remove them. (`Weave.Service.clientsEngine.isMobile` and mobile bookmarks are still very much in use, though. We want to keep those around).
Hi Kit! I just uploaded a WIP patch. I removed the constants `MOBILE_BATCH_SIZE` and `DEFAULT_MOBILE_GUID_FETCH_SIZE`. As for the `client.type` I wasn't sure if I had to remove them completely or only when it's `mobile`. I went with removing when it was `mobile` for this version of the patch. I also need help with the tests, `MOBILE_BATCH_SIZE` appears on these tests: https://dxr.mozilla.org/mozilla-central/source/services/sync/tests/unit/test_history_engine.js https://dxr.mozilla.org/mozilla-central/source/services/sync/tests/unit/test_syncengine_sync.js On this version of the patch, these are the only failing tests under services/sync/ I tried to modify the tests to not use `MOBILE_BATCH_SIZE`, but the removal of `client.type` == `mobile` changes the amount of GET messages and records synced (the things that the tests test). Maybe you can point me in the right direction about this, or to some documentation to better understand what these tests are trying to achieve. Thanks!
Thanks for the patch, Tiago! I think you can remove `test_history_engine.js` `getBatched` takes care of this one level down, and it's covered by other tests. You can also delete the `test_processIncoming_store_toFetch` and `test_processIncoming_mobile_batchSize` tests in `test_syncengine_sync.js`.
Assignee: nobody → tiago.paez11
Status: NEW → ASSIGNED
Priority: -- → P1
Comment on attachment 8870179 [details] Bug 1366081 - Excise 'mobileGUIDFetchBatchSize' and other mobile sync-related constants. https://reviewboard.mozilla.org/r/141622/#review145722 Thanks! Please fix up the tests, and we should be good to land.
Attachment #8870179 - Flags: review?(kit)
Comment on attachment 8870179 [details] Bug 1366081 - Excise 'mobileGUIDFetchBatchSize' and other mobile sync-related constants. https://reviewboard.mozilla.org/r/141622/#review147272
Attachment #8870179 - Flags: review?(kit) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/1c1bf54915bb Excise 'mobileGUIDFetchBatchSize' and other mobile sync-related constants. r=kitcambridge
You need to log in before you can comment on or make changes to this bug.