From 4ffb7c09cf853bfb5834ce3d8c9989bfda2f68f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niko=20K=C3=B6ser?= Date: Wed, 10 Feb 2021 14:49:25 +0100 Subject: [PATCH 1/5] chore(comment): add punctuation to function comment --- notify.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notify.go b/notify.go index 489e357..692524a 100644 --- a/notify.go +++ b/notify.go @@ -18,7 +18,7 @@ type Notify struct { // ErrSendNotification signals that the notifier failed to send a notification. var ErrSendNotification = errors.New("send notification") -// New returns a new instance of Notify. Defaulting to being not disabled +// New returns a new instance of Notify. Defaulting to being not disabled. func New() *Notify { notifier := &Notify{ Disabled: defaultDisabled, From 236e66e284a9f20c7df13ea711ed752e4e89793e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niko=20K=C3=B6ser?= Date: Wed, 10 Feb 2021 14:51:32 +0100 Subject: [PATCH 2/5] chore(comment): put actual type names into comments --- send.go | 2 +- use.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/send.go b/send.go index 66e8a9d..0a3ac26 100644 --- a/send.go +++ b/send.go @@ -5,7 +5,7 @@ import ( "golang.org/x/sync/errgroup" ) -// Send calls the underlying notification services to send the given message to their respective endpoints. +// Send calls the underlying notification services to send the given subject and message to their respective endpoints. func (n Notify) Send(subject, message string) error { if n.Disabled { return nil diff --git a/use.go b/use.go index 18a8f9e..ca85ca3 100644 --- a/use.go +++ b/use.go @@ -1,6 +1,6 @@ package notify -// useService adds a given service to the notifiers services list. +// useService adds a given service to the Notifier's services list. func (n *Notify) useService(service Notifier) { if service == nil { return @@ -8,7 +8,7 @@ func (n *Notify) useService(service Notifier) { n.notifiers = append(n.notifiers, service) } -// UseServices adds the given service(s) to the notifiers services list. +// UseServices adds the given service(s) to the Notifier's services list. func (n *Notify) UseServices(service ...Notifier) { for _, s := range service { n.useService(s) From b3cad8fe68a0a97c7961d180d45b44cb44c9304c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niko=20K=C3=B6ser?= Date: Wed, 10 Feb 2021 14:52:09 +0100 Subject: [PATCH 3/5] refactor(use): simplify useService function --- use.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/use.go b/use.go index ca85ca3..71c54e4 100644 --- a/use.go +++ b/use.go @@ -2,10 +2,9 @@ package notify // useService adds a given service to the Notifier's services list. func (n *Notify) useService(service Notifier) { - if service == nil { - return + if service != nil { + n.notifiers = append(n.notifiers, service) } - n.notifiers = append(n.notifiers, service) } // UseServices adds the given service(s) to the Notifier's services list. From bd82278932b6a5c950de057d5098f695360c0ba9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niko=20K=C3=B6ser?= Date: Wed, 10 Feb 2021 14:55:42 +0100 Subject: [PATCH 4/5] refactor(notifier): remove AddReceivers function from notify.Notifier interface AddReceivers was added to the notify.Notifier interface in 3d7cc206e13d7149c0e9750d257f43abd276b1f4 and the fact that it shouldn't be there was obviously overlooked during the code review. The AddReceivers function is present in all services so far, but it does not define the behavior of our services. The behavior is defined only by the Send function. The AddReceivers function is also not called once in the code on a notify.notifier. This can also be seen by the fact that I can remove the AddReceivers function from the notify.Notifier interface without getting any errors in the rest of the code. --- notifier.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/notifier.go b/notifier.go index 7296054..09dcf51 100644 --- a/notifier.go +++ b/notifier.go @@ -1,14 +1,9 @@ package notify -// Notifier defines the behavior for notification services. It implements Send and AddReciever +// Notifier defines the behavior for notification services. // -// The Send command simply sends a message string to the internal destination Notifier. -// E.g for telegram it sends the message to the specified group chat. -// -// The AddReceivers takes one or many strings and -// adds these to the list of destinations for receiving messages -// e.g. slack channels, telegram chats, email addresses. +// The Send function simply sends a subject and a message string to the internal destination Notifier. +// E.g for telegram.Telegram it sends the message to the specified group chat. type Notifier interface { Send(string, string) error - AddReceivers(...string) } From ecdfb6f1cf64ff751d7249c66bd5cd74570b7d58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niko=20K=C3=B6ser?= Date: Wed, 10 Feb 2021 15:11:32 +0100 Subject: [PATCH 5/5] refactor(telegram): simplify telegram.AddReceivers function Since AddReceivers no longer belongs to the notify.Notifier interface its no longer forced to use a variadic string array as its main parameter which needed to be parsed to an int64 array internally. The telegram.AddReceivers now takes a variadic int64 array as parameter. --- service/telegram/telegram.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/service/telegram/telegram.go b/service/telegram/telegram.go index 4c4528f..0fa4484 100644 --- a/service/telegram/telegram.go +++ b/service/telegram/telegram.go @@ -1,8 +1,6 @@ package telegram import ( - "strconv" - tgbotapi "github.com/go-telegram-bot-api/telegram-bot-api" "github.com/pkg/errors" ) @@ -34,13 +32,8 @@ func New(apiToken string) (*Telegram, error) { // AddReceivers takes Telegram chat IDs and adds them to the internal chat ID list. The Send method will send // a given message to all those chats. -func (t *Telegram) AddReceivers(chatIDs ...string) { - for _, v := range chatIDs { - chatID, err := strconv.ParseInt(v, 10, 64) - if err == nil { - t.chatIDs = append(t.chatIDs, chatID) - } - } +func (t *Telegram) AddReceivers(chatIDs ...int64) { + t.chatIDs = append(t.chatIDs, chatIDs...) } // Send takes a message subject and a message body and sends them to all previously set chats. Message body supports