Code review comment for lp://staging/~ev/daisy/weighted-tests

Revision history for this message
Matthew Paul Thomas (mpt) wrote :

> + # On the first day we had any error reports, the weighting would be 0
> + # because 0 days have past since the first report.

"passed"

> + # The second report is one day after the first and the only report of
> + # the day.
> + self.assertEqual(weights[timestamps[1] / 1e6], 1/90.0)
> +
> + # The third report is two days after the first and the only report of
> + # the day.
> + self.assertEqual(weights[timestamps[2] / 1e6], 2/90.0)

Commas after "first" would make these comments easier to understand.

> + working_date = target_date - datetime.timedelta(days=89)
>...
> + adj = min(day_difference, 90) / 90.0

These lines are far apart from each other and not obviously related. So if later we decide to change the ramp-up to 30 days, for example (improving responsiveness at the expense of spikes), you or someone else might easily change the latter while forgetting the former. I suggest using the same constant in both lines (e.g. datetime.timedelta(days=RAMPUP-1)), and including a comment explaining what it's for.

review: Needs Fixing

« Back to merge proposal