[Merge] lp:~sergiusens/account-polld/account-polld intolp:account-polld

[Sergio Schvezov]

On lunes 14 de julio de 2014 07h'27:58 ART, James Henstridge wrote:
> Review: Needs Fixing
>
> This looks like the right idea in general, but there are a few 
> things that need fixing.  See the inline comments.
>
> Diff comments:
>
>> === modified file 'cmd/account-polld/account.go'
>> --- cmd/account-polld/account.go	2014-07-09 03:13:25 +0000
>> +++ cmd/account-polld/account.go	2014-07-13 23:39:14 +0000
>> @@ -21,39 +21,29 @@
>> "log"
>  ...
>
> You should check data.Enabled here: if an account service is 
> flipped to the disabled state, the corresponding poll goroutine 
> should stop.  If we keep the Account struct around but in a 
> stopped state, we also need a way to start it again.

Right, I have this and checking/listening for the installation state of te 
APP_ID that's supposed to handle the notification; I intend to stop on 
both.

> Also, "data" is going to be changed each time through this 
> loop, so storing "&data" in the account is going to result in 
> every account poller using the tokens for the most recent 
> account.

Hmm, I might be missing something; I storing an '*Account' in the accounts 
map with the keys here for the AccountId; I'll take a closer look in case 
missed something.

I should probably change the Accounts type to a different name to avoid 
confusions as well.

> Either use a pointer to a copy of data, or embed the 
> accounts.AuthData struct directly.  Either way, we probably need 
> some way to synchronise the update with the account.Loop 
> goroutine.
>
>> +		} else {
>> +			var plugin plugins.Plugin
>> +			switch data.ServiceName {
>> +			case SERVICENAME_GMAIL:
>> +				log.Println("Creating account with id", data.AccountId, 
>> "for", data.ServiceName)
>  ...
>
>


-- 
https://code.launchpad.net/~sergiusens/account-polld/account-polld/+merge/226620
Your team Ubuntu Phablet Team is subscribed to branch lp:account-polld.