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.

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

RUMM-1272 Implement a VitalObserver #493

Merged
merged 2 commits into from
May 20, 2021
Merged

Conversation

xgouchet
Copy link
Member

What and why?

Implement a way to aggregate a series of values over time.

How?

We create a ValueObserver implementation that will keep the min, max and mean value as well as a sample count.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference

@xgouchet xgouchet requested a review from a team as a code owner May 20, 2021 06:58
Copy link
Member

@mariusc83 mariusc83 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@ncreated ncreated left a comment

Choose a reason for hiding this comment

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

Looks good 👌

@ncreated
Copy link
Member

ncreated commented May 20, 2021

You may need to install swiftlint to fix linter issues found on CI (with swiftlint installed, issues will be listed in Xcode as build warnings). It should be normally installed for you when you did make dependencies, it's weird if it's not 🤔.

@xgouchet xgouchet force-pushed the xgouchet/RUMM-1272/VitalMonitor branch 2 times, most recently from 94e843e to 3089ae5 Compare May 20, 2021 09:55
@xgouchet xgouchet force-pushed the xgouchet/RUMM-1272/VitalMonitor branch from 3089ae5 to 6bda585 Compare May 20, 2021 10:20
@xgouchet xgouchet merged commit a5ac373 into master May 20, 2021
@xgouchet xgouchet deleted the xgouchet/RUMM-1272/VitalMonitor branch May 20, 2021 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants