Day Scope

Merge lp://staging/~day-scope-team/day-scope/fixes-1500486-cache-sun-info into lp://staging/day-scope

  1. fixes-1500486-cache-sun-info
  2. Merge into trunk
Proposed by Kyle Nitzsche
Status: Merged
Merged at revision: 41
Proposed branch: lp://staging/~day-scope-team/day-scope/fixes-1500486-cache-sun-info
Merge into: lp://staging/day-scope
Diff against target: 433 lines (+186/-70)
4 files modified
CMakeLists.txt (+2/-2)
include/query.h (+10/-4)
src/CMakeLists.txt (+6/-6)
src/query.cpp (+168/-58)
To merge this branch: bzr merge lp://staging/~day-scope-team/day-scope/fixes-1500486-cache-sun-info
Reviewer Review Type Date Requested Status
Jin (community) Approve
Review via email: mp+285126@code.staging.launchpad.net

Description of the change

cache sunrise/sunset info and use it:
* if it was retrieved today
* and if we have network and location. If we don't have those, show "No Information" (localized).

This should speed up Day scope and therefore speed up Today scope.

To post a comment you must log in.
lp://staging/~day-scope-team/day-scope/fixes-1500486-cache-sun-info updated
42. By Kyle Nitzsche

when the lunar phase is not translated, it needs a space inserted so, for
example, "fullmoon" becomse "Full moon". This commit does this for the
eight phases returned by the web api

43. By Kyle Nitzsche

modified fix to change the moon phase names AFTER its translation has been
retrieved, thus it does not break translations.

Revision history for this message
Jin (jindallo) :
Revision history for this message
Jin (jindallo) :
Revision history for this message
Jin (jindallo) :
Revision history for this message
Jin (jindallo) wrote :

Generally, the code looks good to me,
but some suggestions as below,
please take a look there,
thank you.

Revision history for this message
Jin (jindallo) :
review: Approve
lp://staging/~day-scope-team/day-scope/fixes-1500486-cache-sun-info updated
44. By Kyle Nitzsche

combine useNetworkSuns() and useNetworkLunar() into single method:
useNetwork() and pass an enum to control which code runs inside

45. By Kyle Nitzsche

combine getLunarFromNetwork() and getSunsFromNetwork() into single method:
getFromNetwork and use passed enum to control code path

46. By Kyle Nitzsche

combine getLunarFromLocal() and getSunsFromLocal() into:
getFromLocal() and use passed data_t to specify code path

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

Hi Jin,

I've combined three pairs of lunar/sun methods into single methods that now handle both cases controlled by the passed enum arg as you requested:

  combine getLunarFromLocal() and getSunsFromLocal() into:
  getFromLocal() and use passed data_t to specify code path

  combine getLunarFromNetwork() and getSunsFromNetwork() into single method:
  getFromNetwork and use passed enum to control code path

  combine useNetworkSuns() and useNetworkLunar() into single method:
  useNetwork() and pass an enum to control which code runs inside

Revision history for this message
Jin (jindallo) wrote :

Hello Kyle,

Oh super!

The code looks great to me,
I can't wait for this landing!

Many thanks.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: