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

Update watchdog dependency #540

Merged
merged 5 commits into from
Apr 10, 2024
Merged

Update watchdog dependency #540

merged 5 commits into from
Apr 10, 2024

Conversation

ankona
Copy link
Contributor

@ankona ankona commented Apr 4, 2024

Updated watchdog dependency pin to next major version and removed type: ignore where possible due to new type hints added to watchdog

Copy link

codecov bot commented Apr 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.70%. Comparing base (505de50) to head (c61f478).
Report is 1 commits behind head on develop.

❗ Current head c61f478 differs from pull request most recent head e7eb2ce. Consider uploading reports for the commit e7eb2ce to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #540      +/-   ##
===========================================
- Coverage    90.76%   90.70%   -0.07%     
===========================================
  Files           65       65              
  Lines         4528     4498      -30     
===========================================
- Hits          4110     4080      -30     
  Misses         418      418              
Files Coverage Δ
smartsim/_core/utils/telemetry/telemetry.py 90.57% <100.00%> (ø)

... and 3 files with indirect coverage changes

@ankona ankona marked this pull request as ready for review April 4, 2024 18:24
@ankona ankona requested a review from MattToast April 4, 2024 18:24
@mellis13 mellis13 requested a review from AlyssaCote April 4, 2024 18:25
Copy link
Contributor

@AlyssaCote AlyssaCote left a comment

Choose a reason for hiding this comment

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

This looks great! The only thing holding up the approval for me is I'm curious why the self._observer starts and stops don't have their # type: ignore comments removed because it looks like BaseObserver is a watchdog class.

Answered offline. Should be good!

@AlyssaCote AlyssaCote self-requested a review April 9, 2024 20:22
Copy link
Contributor

@AlyssaCote AlyssaCote left a comment

Choose a reason for hiding this comment

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

Thanks for answering my lingering question!

@ankona ankona merged commit 1267a9a into CrayLabs:develop Apr 10, 2024
42 checks passed
@amandarichardsonn amandarichardsonn added area: third-party Issues related to Issues related to dependencies and third-part and third-party package integrations type: refactor Issues focused on refactoring existing code bug: minor A minor bug non-user-facing labels Apr 25, 2024
@ankona ankona deleted the 478 branch June 3, 2024 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: third-party Issues related to Issues related to dependencies and third-part and third-party package integrations bug: minor A minor bug non-user-facing type: refactor Issues focused on refactoring existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants