Code review comment for lp:~jtatum/ubuntu-accomplishments-daemon/lp-caching-part-1

Revision history for this message
Rafał Cieślak (rafalcieslak256) wrote :

Thanks for your work, James. I really like the idea of simplifying scripts, it seems you have done a pretty good job here.
However, as you have mentioned, it seems that these scripts will make sense only for the ubuntu-community collection. Could this code be somehow bundled with the accomplishments scripts within that collection, to avoid adding to the daemon pieces of code that are meant to be useful only for a particular collection? If yes, then it would make much more sense if this was included in that package instead of the daemon.

There is one more thing that worries me - the cache is not updated. Imagine such situation:
1. User A is not a Ubuntu Member.
2. The cache is created for him, it remembers that is is not a Member.
3. He reads the instructions on how to become a Member, and soon accomplishes this trophy.
4. User A clicks 'check-accomplishment', or waits up to 15 minutes for the scripts to be run.
5. The daemon uses scripts to check if Ubuntu Member has been accomplished.
6. The scripts ask the cache if he is a member of 'ubuntu-members'
7. Obviously, the cache denies that, and so the user is unable to gain this trophy (and similarly a lot of others).

« Back to merge proposal