1039465 - [Contacts DataRefactor] Tweak global_contacts_data module and multi_contact

Status: RESOLVED FIXED

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

Description

[Jose Manuel Cantera]

When dealing with performance test We discovered that there are some issues in global contacts and multi contact modules. We need to fix them.

Comment 1

[Jose Manuel Cantera]

Attachment: 21872.html

Comment 2

[Francisco Jordano [:arcturus] [:francisco]]

I'm a bit overloaded, Sergi would you mind to take a look?

Comment 3

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Comment on attachment 8457999 [details]
21872.html

Reviewed. I wrote some suggestions and a couple of questions about the tests.

Comment 4

[Jose Manuel Cantera]

Sergi,

please could you have a look. The PR has been there for a long time

thanks!

Comment 5

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

I added some comments to the review, please assign to me again when it's ready, or let's discuss. Looks good so far!

Comment 6

[Jose Manuel Cantera]

Comment on attachment 8457999 [details]
21872.html

Sergi, 

I have commented the comments you made. Please have a look 

thanks

Comment 7

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Hi Jose,

I agree with most comments. The only ones left to answer are:

https://github.com/mozilla-b2g/gaia/pull/21872/files#diff-0a65f7b5fa64c80d46a8f7fd40d8109cR627

Also, the question:
What is the difference of using the store variable in your code and using getDataStore()? They return the same object, right? In that case, only one of them should be used throughout the code.

Comment 8

[Jose Manuel Cantera]

(In reply to Sergi Mansilla [:sergi] (Telenor) from comment #7)
> Hi Jose,
> 
> I agree with most comments. The only ones left to answer are:
> 
> https://github.com/mozilla-b2g/gaia/pull/21872/files#diff-
> 0a65f7b5fa64c80d46a8f7fd40d8109cR627
> 

I've responded, thanks

> Also, the question:
> What is the difference of using the store variable in your code and using
> getDataStore()? They return the same object, right? In that case, only one
> of them should be used throughout the code.

we need to call it to ensure the datastore is properly initialized. fixing that thanks

Comment 9

[Jose Manuel Cantera]

(In reply to Jose Manuel Cantera from comment #8)
> (In reply to Sergi Mansilla [:sergi] (Telenor) from comment #7)
> > Hi Jose,
> > 
> > I agree with most comments. The only ones left to answer are:
> > 
> > https://github.com/mozilla-b2g/gaia/pull/21872/files#diff-
> > 0a65f7b5fa64c80d46a8f7fd40d8109cR627
> > 
> 
> I've responded, thanks
> 
> > Also, the question:
> > What is the difference of using the store variable in your code and using
> > getDataStore()? They return the same object, right? In that case, only one
> > of them should be used throughout the code.
> 
> we need to call it to ensure the datastore is properly initialized. fixing
> that thanks

Update: We need to call getDatastore when we cannot guarantee that the datastore had been obtained previously. however, there are code paths on which we know the datastore must be there, so in order to avoid complexity, we use the variable directly. 

I hope this clarifies

Comment 10

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Comment on attachment 8457999 [details]
21872.html

Thanks Jose! r+

Comment 11

[Jose Manuel Cantera]

landed in master:

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