TRTemperatureComparison+localizedStringFromComparison accepts a date #144
Conversation
jakecraige
force-pushed
the
jc-comparison-text-from-cache
branch
from
9b15707
to
eb8735c
Compare
|
honestly this isn't that bad. I think it makes more sense, especially readying the function signatures. My vote would be for this instead of #133 |
jakecraige
force-pushed
the
jc-comparison-text-from-cache
branch
from
eb8735c
to
d40310e
Compare
|
@tonyd256 @gfontenot Can I get one another once-over / review on this? Cleaned up the merge conflicts. Tests are good locally. |
|
|
||
| SpecBegin(TRTemperatureComparisonFormatter) | ||
|
|
||
| NSDate* (^dateFromString) (NSString*) = ^NSDate* (NSString *dateString) { |
There was a problem hiding this comment.
Can you move these pointers to the other side of the space? (NSDate *( instead of NSDate* ()
There was a problem hiding this comment.
Actually, we should have Quick/Nimble/Swift in master at this point, right? Maybe we can test this whole thing in Swift and not have to deal with this at all?
There was a problem hiding this comment.
No problem on the pointers.
Because of the way I'm testing this right now I don't think we can. To keep a consistent time zone in test I've made a category up on LN4 to expose the private calendar, then on LN21 I set it's time zone to a specific one. There's likely other ways to accomplish this but for now this does the trick. We can probably set up the class a little better for testing when converting it to Swift.
|
one comment re: testing, but this looks good to me. |
There was a bug in where comparisons shown from the cache would show inaccurate text because it wasn't using the date from the cache. Ex: If the date was cached in the AM and you open the app in the afternoon it might say, "Last updated 8:00AM, It's the same this afternoon as yesterday afternoon." when it should say, "Last updated 8:00AM, It's the same this morning as yesterday morning." This PR has the comparison be initialized with a date that it uses when checking the time of day. https://trello.com/c/IeLJK4C8/36-temperature-comparison-text-not-taking-cached-date-into-account
jakecraige
force-pushed
the
jc-comparison-text-from-cache
branch
from
d40310e
to
ed9fd5b
Compare
|
There was a bug in where comparisons
shown from the cache would show inaccurate text
because it wasn't using the date from the cache.
Ex:
If the date was cached in the AM and you open the app in the afternoon it might say,
"Last updated 8:00AM, It's the same this afternoon as yesterday afternoon."
when it should say,
"Last updated 8:00AM, It's the same this morning as yesterday morning."
This PR has the comparison be initialized with a date
that it uses when checking the time of day.
I chose to switch from a class method to instance method
so that I didn't have to pass the date
through a chain of class methods.
https://trello.com/c/IeLJK4C8/36-temperature-comparison-text-not-taking-cached-date-into-account