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

ref: Make sure both base status notifiers cache notifications #90

Merged
merged 2 commits into from
Sep 7, 2023

Conversation

scott-codecov
Copy link
Contributor

The CheckNotifier inherits from StatusNotifier which is where notification caching was happening. CheckNotifier overrides the notify method but does not implement the caching. This PR refactors where the caching happens such that it can easily be reused in both base notifier classes.

@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Merging #90 (e42e491) into main (4a4bb80) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

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

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #90      +/-   ##
==========================================
- Coverage   98.47%   98.47%   -0.01%     
==========================================
  Files         362      362              
  Lines       26517    26494      -23     
==========================================
- Hits        26113    26090      -23     
  Misses        404      404              
Flag Coverage Δ
integration 98.44% <100.00%> (-0.01%) ⬇️
latest-uploader-overall 98.44% <100.00%> (-0.01%) ⬇️
onlysomelabels 98.47% <100.00%> (-0.01%) ⬇️
unit 98.44% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
NonTestCode 97.13% <100.00%> (-0.01%) ⬇️
OutsideTasks 98.24% <100.00%> (-0.01%) ⬇️
Files Changed Coverage Δ
services/notification/notifiers/checks/base.py 98.22% <100.00%> (ø)
services/notification/notifiers/status/base.py 98.69% <100.00%> (+0.01%) ⬆️

... and 9 files with indirect coverage changes

Related Entrypoints
run/app.tasks.notify.Notify

📢 Have feedback on the report? Share it here.

@scott-codecov scott-codecov merged commit c828650 into main Sep 7, 2023
7 of 8 checks passed
@scott-codecov scott-codecov deleted the scott/refactor-notifier-caching branch September 7, 2023 19:45
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.

2 participants