1215473 - Avoid accessing private variable placesStore in history DataAdapter

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Sync, defect, P3)

Description

[Michiel de Jong [:michielbdejong]]

At this line [1] and also two lines below, `store` is not defined.

[1] https://github.com/mozilla-b2g/gaia/blob/4d1ff2b17850c3ec3d9c6869b73e9a45e4338db4/apps/sync/js/adapters/bookmarks.js#L123

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] michielbdejong:1215473-store-variable-in-scope > mozilla-b2g:master

Comment 2

[Michiel de Jong [:michielbdejong]]

Needs a unit test, still.

Comment 3

[Michiel de Jong [:michielbdejong]]

The reason this is currently working is that ensureStore uses a variable that's also called 'store'. We could leave this as it is, but I think it's cleaner to rename the private variable to _store, and force the rest of the code to always use the value provided ensureStore, instead of sometimes accessing the private variable directly.

Comment 4

[Sean Lee [:seanlee][:weilonge]]

Comment on attachment 8674818 [details] [review]
[gaia] michielbdejong:1215473-store-variable-in-scope > mozilla-b2g:master

Hello Michiel, I leave some comments at github. BTW, how do you think if keeping store as a local variable in addBookmark function?
Please f? or r? me again once you have any update. Thank you!

Comment 5

[Sean Lee [:seanlee][:weilonge]]

I am going to implement this in bug 1215086.

Comment 6

[Michiel de Jong [:michielbdejong]]

Great!

Comment 7

[Sean Lee [:seanlee][:weilonge]]

Comment on attachment 8674818 [details] [review]
[gaia] michielbdejong:1215473-store-variable-in-scope > mozilla-b2g:master

LGTM :)

Comment 8

[Michiel de Jong [:michielbdejong]]

Marking this as P3 / Sprint 11 because it's code cleanup and doesn't affect functionality.

Comment 9

[Michiel de Jong [:michielbdejong]]

Rebased.

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

https://github.com/mozilla-b2g/gaia/commit/f8cc75ad7106f52d3835c4e697062fc6ed6c1dfa