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): title customization #1219

Merged
merged 4 commits into from
Apr 18, 2022
Merged

fix(notifications): title customization #1219

merged 4 commits into from
Apr 18, 2022

Conversation

piksel
Copy link
Member

@piksel piksel commented Feb 11, 2022

This PR introduces two new flags to watchtower:
--notification-skip-title
and
--notification-title-tag <TAG> (with a backwards compatible alias to notification-email-subjecttag).

The title-tag flag works the same way as the email-subjecttag used to work. If supplied, it will prefix the titles for all notifications with [<TAG>] . Since it is generic among all notifications (which support titles), it was renamed to reflect this, but using the old name still works for now. Fixes #1205

Supplying the skip-title flag will not override the title param for the shoutrrr service, so it can still be set to whatever value you want using the &title= query parameter of the shoutrrr URL. This should in effect remove the title if no such query param is supplied. Fixes #1207

This was done in a combined effort to simplify the title generation and fixing the unintended breaking changes in 1.4.0

@codecov
Copy link

codecov bot commented Feb 11, 2022

Codecov Report

Merging #1219 (2407452) into main (a5c60a9) will increase coverage by 0.60%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1219      +/-   ##
==========================================
+ Coverage   63.00%   63.60%   +0.60%     
==========================================
  Files          23       23              
  Lines        1500     1522      +22     
==========================================
+ Hits          945      968      +23     
+ Misses        471      470       -1     
  Partials       84       84              
Impacted Files Coverage Δ
internal/flags/flags.go 89.08% <100.00%> (+0.26%) ⬆️
pkg/notifications/notifier.go 84.93% <100.00%> (+4.93%) ⬆️
pkg/notifications/shoutrrr.go 74.69% <100.00%> (+0.30%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5c60a9...2407452. Read the comment docs.

@piksel piksel marked this pull request as ready for review February 11, 2022 10:13
@piksel piksel requested a review from simskij as a code owner February 11, 2022 10:13
Copy link
Member

@simskij simskij left a comment

Choose a reason for hiding this comment

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

I'm 💯 fine with merging this as-is but left a minor comment.

Comment on lines +247 to +250
type StaticData struct {
Title string
Host string
}
Copy link
Member

Choose a reason for hiding this comment

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

Could this naming be improved to clarify what it is? StaticData is very generic.

Copy link
Member Author

@piksel piksel Mar 21, 2022

Choose a reason for hiding this comment

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

It is, but it's the best way I found to describe it. It's the non-changing (static) part of the data used for notifications. It could perhaps be called StaticTemplateData, but it's not, strictly speaking, only used for that.
Any suggestions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants