Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make baseline comparison look backwards #84

Merged
merged 1 commit into from
Mar 5, 2018
Merged

Make baseline comparison look backwards #84

merged 1 commit into from
Mar 5, 2018

Conversation

remybach
Copy link
Contributor

@remybach remybach commented Mar 5, 2018

This better matches up with how relTime works as far as I understand it.

@remybach remybach requested review from adgad and kitkat119 March 5, 2018 13:11
@adgad
Copy link
Contributor

adgad commented Mar 5, 2018

I'm not sure how relTime works, but I think the intention of this was if, for example, if you have a timeframe of 2 days (say Mon/Tue) and you specify 7 as your baselinePeriod, then you'd want to compare Mon/Tue of the previous week.

With this change you'd have to specify 9 as your baselinePeriod to get the same, which feels a bit counterintuitive?

adgad
adgad approved these changes Mar 5, 2018
View reviewed changes
Copy link
Contributor

@adgad adgad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense once we did the math

@remybach remybach merged commit dde3ca3 into master Mar 5, 2018
@remybach remybach deleted the rb-keen-spike branch March 5, 2018 15:32
JSRedondo pushed a commit that referenced this pull request Jun 20, 2022
@remybach
Make baseline comparison look backwards (#84)
e732c34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adgad adgad adgad approved these changes

@kitkat119 kitkat119 Awaiting requested review from kitkat119

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@remybach @adgad