Skip to content

Commit

Permalink
Address change requests
Browse files Browse the repository at this point in the history
  • Loading branch information
travis79 committed Jan 16, 2020
1 parent 4b90ad3 commit 0cf4b24
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 7 deletions.
13 changes: 7 additions & 6 deletions glean-core/ios/Glean/Scheduler/MetricsPingScheduler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,11 @@ class MetricsPingScheduler {

// We expect to cover 3 cases here:
//
// (1) - the ping was already collected the current calendar day; only schedule
// (1) - the ping was already collected on the current calendar day; only schedule
// one for collecting the next calendar day at the due time;
// (2) - the ping was NOT collected the current calendar day, and we're later than
// (2) - the ping was NOT collected on the current calendar day, and we're later than
// the due time; collect the ping immediately;
// (3) - the ping was NOT collected the current calendar day, but we still have
// (3) - the ping was NOT collected on the current calendar day, but we still have
// some time to the due time; schedule for sending the current calendar day.

let alreadySentToday = lastSentDate != nil && Calendar.current.isDateInToday(lastSentDate!)
Expand All @@ -136,9 +136,10 @@ class MetricsPingScheduler {
// engines through the `Dispatchers.API` context, so this ensures we are enqueued
// before any other recording API call.
//
// * Do not change `Dispatchers.shared.serialOperationQueue.addOperation` to
// - Do not change `Dispatchers.shared.serialOperationQueue.addOperation` to
// `Dispatchers.shared.launchAPI` as this would break startup overdue ping
// collection. * `addOperation` schedules the task for immediate execution on the
// collection.
// - `addOperation` schedules the task for immediate execution on the
// `Dispatchers` serial execution queue, before any other enqueued task. For more
// context, see bug 1604861 and the implementation of
// `collectPingAndReschedule`.
Expand Down Expand Up @@ -186,7 +187,7 @@ class MetricsPingScheduler {

/// Get the date the metrics ping was last collected.
///
/// - returns: An Optional `Date` representing the date the metrics ping was last collected, or null if no metrics
/// - returns: A `Date` representing the when the metrics ping was last collected, or nil if no metrics
/// ping was previously collected.
func getLastCollectedDate() -> Date? {
var lastCollectedDate: Date?
Expand Down
2 changes: 1 addition & 1 deletion glean-core/ios/Glean/Utils/Utils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ extension Date {
/// Convenience function to convert a Date to an ISO8601 string
///
/// - returns: An ISO8601 `String?` representing the current value of the `Date` object
func toISO8601String(precision: TimeUnit) -> String? {
func toISO8601String(precision: TimeUnit) -> String {
let dateFormatter = DateFormatter()
dateFormatter.dateFormat = dateFormatPatterns[precision]
return dateFormatter.string(from: self)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class MetricsPingSchedulerTests: XCTestCase {
fakeNow.month = 6
fakeNow.day = 11
fakeNow.hour = 3
fakeNow.minute = 0
fakeDate = Calendar.current.date(from: fakeNow)!
XCTAssertFalse(
mps.isAfterDueTime(fakeDate, dueHourOfTheDay: 4),
Expand All @@ -71,6 +72,7 @@ class MetricsPingSchedulerTests: XCTestCase {
fakeNow.month = 6
fakeNow.day = 11
fakeNow.hour = 4
fakeNow.minute = 0
fakeDate = Calendar.current.date(from: fakeNow)!
XCTAssertFalse(
mps.isAfterDueTime(fakeDate, dueHourOfTheDay: 4),
Expand All @@ -81,6 +83,7 @@ class MetricsPingSchedulerTests: XCTestCase {
fakeNow.month = 6
fakeNow.day = 11
fakeNow.hour = 0
fakeNow.minute = 0
fakeDate = Calendar.current.date(from: fakeNow)!
XCTAssertFalse(
mps.isAfterDueTime(fakeDate, dueHourOfTheDay: 4),
Expand Down

0 comments on commit 0cf4b24

Please sign in to comment.