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

Add daily active deposits metric #1013

Merged
merged 1 commit into from
Mar 14, 2019
Merged

Add daily active deposits metric #1013

merged 1 commit into from
Mar 14, 2019

Conversation

kalkov
Copy link
Member

@kalkov kalkov commented Mar 1, 2019

Summary

@kalkov kalkov force-pushed the add-daily-active-deposits-metric branch 3 times, most recently from 916f83f to 671c0af Compare March 5, 2019 12:42
@kalkov kalkov marked this pull request as ready for review March 5, 2019 13:01
@kalkov kalkov force-pushed the add-daily-active-deposits-metric branch from 671c0af to 20c7522 Compare March 7, 2019 14:30
@kalkov kalkov force-pushed the add-daily-active-deposits-metric branch from 20c7522 to 8ce7348 Compare March 12, 2019 15:47
@tspenov
Copy link
Contributor

tspenov commented Mar 12, 2019

👍 LGTM - small comment in test

IvanIvanoff
IvanIvanoff reviewed Mar 12, 2019
View reviewed changes
lib/sanbase/clickhouse/daily_active_deposits.ex Outdated
GROUP BY time
)
GROUP BY dt
ORDER BY dt
Copy link
Member

Choose a reason for hiding this comment

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

I am a little bit confused by the redefinitions of dt and time. Is it possible to do something like toUnixTimestamp(time) as time?

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed, please check the fix

@kalkov kalkov force-pushed the add-daily-active-deposits-metric branch from e6521f0 to b73eb43 Compare March 14, 2019 12:55
@kalkov kalkov merged commit b73eb43 into master Mar 14, 2019
@kalkov kalkov deleted the add-daily-active-deposits-metric branch March 14, 2019 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tspenov tspenov tspenov approved these changes

@IvanIvanoff IvanIvanoff IvanIvanoff approved these changes

@valo valo Awaiting requested review from valo

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

Successfully merging this pull request may close these issues.
4 participants
@kalkov @tspenov @valo @IvanIvanoff