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

fix(notifications): correctly set the delay from options #1726

Merged
merged 1 commit into from
Aug 12, 2023

Conversation

Tentoe
Copy link
Contributor

@Tentoe Tentoe commented Aug 11, 2023

--notifications-delay was ignored by soutrrr notifications because it wasn't set on creation.

I tested with dockerfiles/Dockerfile.self-contained


Caused by #1377
Probably solves #1485 (needs confirmation)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congratulations on opening your first pull request! We'll get back to you as soon as possible. In the meantime, please make sure you've updated the documentation to reflect your changes and have added test automation as needed. Thanks! 🙏🏼

@codecov
Copy link

codecov bot commented Aug 12, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% 🎉

Comparison is base (7948242) 67.69% compared to head (5b2e548) 67.70%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1726      +/-   ##
==========================================
+ Coverage   67.69%   67.70%   +0.01%     
==========================================
  Files          26       26              
  Lines        2377     2378       +1     
==========================================
+ Hits         1609     1610       +1     
  Misses        670      670              
  Partials       98       98              
Files Changed Coverage Δ
pkg/notifications/shoutrrr.go 74.82% <100.00%> (+0.17%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@piksel piksel left a comment

Choose a reason for hiding this comment

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

Indeed. This even shows up as a warning in goland, I wonder why Codacy didn't catch this... Thanks for spotting it!

@piksel piksel changed the title delay in soutrrr wasn't set so it wasn't observed fix(notifications): correctly set the delay from options Aug 12, 2023
@piksel piksel merged commit 30280e3 into containrrr:main Aug 12, 2023
12 checks passed
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