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

Classes for making standard monitor functions #114

Merged
merged 22 commits into from
Jan 14, 2025
Merged

Classes for making standard monitor functions #114

merged 22 commits into from
Jan 14, 2025

Conversation

jwallwork23
Copy link
Member

@jwallwork23 jwallwork23 commented Aug 26, 2024 •
edited
Loading

Closes #96.
Blocked by #95.

@jwallwork23 jwallwork23 added the enhancement New feature or request label Aug 26, 2024
@jwallwork23 jwallwork23 self-assigned this Aug 26, 2024
@jwallwork23 jwallwork23 mentioned this pull request Aug 26, 2024
Closed
@jwallwork23 jwallwork23 marked this pull request as ready for review December 28, 2024 13:59
@jwallwork23 jwallwork23 requested a review from ddundo January 10, 2025 10:33
@ddundo ddundo mentioned this pull request Jan 12, 2025
Closed
ddundo
ddundo requested changes Jan 12, 2025
View reviewed changes
Copy link
Member

@ddundo ddundo left a comment

Choose a reason for hiding this comment

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

Thanks @jwallwork23! Quite a few comments (I don't know why I hyperfocused on this so much :) ) but they're minor.

movement/monitor.py Outdated Show resolved Hide resolved
movement/monitor.py Outdated Show resolved Hide resolved
movement/monitor.py Outdated Show resolved Hide resolved
movement/monitor.py Outdated Show resolved Hide resolved
movement/monitor.py Outdated Show resolved Hide resolved
movement/monitor.py Outdated Show resolved Hide resolved
movement/monitor.py Outdated Show resolved Hide resolved
movement/monitor.py Outdated Show resolved Hide resolved
movement/monitor.py Outdated Show resolved Hide resolved
movement/monitor.py Outdated Show resolved Hide resolved
@jwallwork23 jwallwork23 requested a review from ddundo January 13, 2025 17:35
@jwallwork23
Copy link
Member Author

Thanks for the very thorough review @ddundo! I think I've responded to your concerns.

ddundo
ddundo approved these changes Jan 13, 2025
View reviewed changes
Copy link
Member

@ddundo ddundo left a comment

Choose a reason for hiding this comment

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

Thanks! There is just one minor unresolved thing left (updating the callback) so I'll approve so you can merge whenever.

@jwallwork23 jwallwork23 mentioned this pull request Jan 14, 2025
Open
@jwallwork23 jwallwork23 merged commit 9250565 into main Jan 14, 2025
1 check passed
@jwallwork23 jwallwork23 deleted the 96_monitor-utils branch January 14, 2025 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ddundo ddundo ddundo approved these changes

Assignees

@jwallwork23 jwallwork23

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Utils for standard monitor functions
2 participants
@jwallwork23 @ddundo