-
Notifications
You must be signed in to change notification settings - Fork 973
Conversation
It could cause periodic jank after the first 2 minutes, and then again every hour after that Fix #9996
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment left- otherwise looks good 😄
@@ -131,17 +131,16 @@ var requestVersionInfo = (done, pingOnly) => { | |||
if (!platformBaseUrl) throw new Error('platformBaseUrl not set') | |||
|
|||
// Get the daily, week of year and month update checks | |||
var lastCheckYMD = AppStore.getState().toJS().updates['lastCheckYMD'] || null | |||
const state = AppStore.getState() | |||
var lastCheckYMD = state.getIn(['updates', 'lastCheckYMD']) || null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update this to use the default value format? ex:
var lastCheckYMD = state.getIn(['updates', 'lastCheckYMD'], null)
might as well make the variables const
too instead of var
😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, addressed here: 05781e4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged it all the way up to 0.17.x but obviously no respin for that, just merged it up for the sake of easier merges if anything else changes.
Looks good to me |
It could cause periodic jank after the first 2 minutes, and then again every hour after that
Fix #9996
Submitter Checklist:
git rebase -i
to squash commits (if needed).Test Plan:
Reviewer Checklist:
Tests