GitHub PullRequestReviewCommentEvent at 2014-01-26 11:48:22
Repository: gaia
Actor: lodr
Payload: {"comment":{"url":"https://api.github.com/repos/mozilla-b2g/gaia/pulls/comments/9175476","id":9175476,"diff_hunk":"@@ -102,11 +102,23 @@ var Settings = (function() {\n         closeSettings();\n       });\n \n-      function _updateNextReset(value, old, key, settings) {\n+      function _updateTrackingPeriod(value, old, key, settings) {\n+        var newResetTime = (settings.trackingPeriod === 'weekly') ?\n+                     parseInt(navigator.mozL10n.get('weekStartsOnMonday'), 10) :\n+                     1;\n+        if (settings.resetTime !== newResetTime) {\n+          ConfigManager.setOption({ resetTime: newResetTime });\n+        } else {\n+          updateNextReset(settings.trackingPeriod, settings.resetTime);\n+        }\n+      }\n+\n+      function _updateResetTime(value, old, key, settings) {\n         updateNextReset(settings.trackingPeriod, settings.resetTime);\n       }\n-      ConfigManager.observe('resetTime', _updateNextReset, true);\n-      ConfigManager.observe('trackingPeriod', _updateNextReset, true);\n+\n+      ConfigManager.observe('resetTime', _updateResetTime, true);\n+      ConfigManager.observe('trackingPeriod', _updateTrackingPeriod, true);","path":"apps/costcontrol/js/settings/settings.js","position":23,"original_position":23,"commit_id":"4ed99d191db61b5986f347d211f4e7d332155340","original_commit_id":"4ed99d191db61b5986f347d211f4e7d332155340","user":{"login":"lodr","id":757942,"avatar_url":"https://gravatar.com/avatar/284a643007ef8adc480a171b0a663e86?d=https%3A%2F%2Fidenticons.github.com%2F9caf34cfefc5f1205a071fc278ceda60.png&r=x","gravatar_id":"284a643007ef8adc480a171b0a663e86","url":"https://api.github.com/users/lodr","html_url":"https://github.com/lodr","followers_url":"https://api.github.com/users/lodr/followers","following_url":"https://api.github.com/users/lodr/following{/other_user}","gists_url":"https://api.github.com/users/lodr/gists{/gist_id}","starred_url":"https://api.github.com/users/lodr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lodr/subscriptions","organizations_url":"https://api.github.com/users/lodr/orgs","repos_url":"https://api.github.com/users/lodr/repos","events_url":"https://api.github.com/users/lodr/events{/privacy}","received_events_url":"https://api.github.com/users/lodr/received_events","type":"User","site_admin":false},"body":"When I read this I realized why you changed the names for the former functions but notice it is not true we should `_updateResetTime` as the consequence of a change in `resetTime` in the same way it is not true we should `_updateTrackingPeriod` as a consequence of a change in `trackingPeriod`.\r\n\r\nThe consequence of changing the `resetTime` is to `_updateNextReset` and the consequences of change `trackingPeriod` is to return the `resetTime` to it default value for the tracking period. Do you understand? Ping me if not. ","created_at":"2014-01-26T11:48:22Z","updated_at":"2014-01-26T11:48:22Z","html_url":"https://github.com/mozilla-b2g/gaia/pull/15411#discussion_r9175476","pull_request_url":"https://api.github.com/repos/mozilla-b2g/gaia/pulls/15411","_links":{"self":{"href":"https://api.github.com/repos/mozilla-b2g/gaia/pulls/comments/9175476"},"html":{"href":"https://github.com/mozilla-b2g/gaia/pull/15411#discussion_r9175476"},"pull_request":{"href":"https://api.github.com/repos/mozilla-b2g/gaia/pulls/15411"}}}}