ausocean / cloud

GNU General Public License v3.0
1 stars 1 forks source link

notify: Add options to specify sender, recipient, filter(s), etc. #77

Closed scruzin closed 1 month ago

scruzin commented 1 month ago

It is proposed to change notifier.Init from:

func (n *Notifier) Init(ctx context.Context, projectID string, sender string, store TimeStore) error

to

func (n *Notifier) Init(ctx context.Context, projectID string, option... Option) error where Option would be defined as:

type Option func(*Notifier,string) error

The following four options are envisioned:

Clients may instantiate multiple Notifiers, each configured with its own options.

saxon-milton commented 1 month ago

Nice; the functional options pattern would have to be one of my favourites. It'll make it really easy to add new options to the same interface as we develop further :)

I'm wondering what the string parameter is for in the Option type however ? The way that I have seen this used (and used personally), is where you provide parameters for the option through the factories for the Options e.g.

type Option func(*Notifier) error

func WithSender(sender string) Option {
    return func(n *Notifier) error {
        n.sender = sender
        return nil
    }
}
scruzin commented 1 month ago

You're right. The Option doesn't need to take a string. I was conflating the fact that all option params are of type string.

scruzin commented 1 month ago

With this approach, notify.Send no longer takes a recipient, as the latter is now specified as an option to notify.Init. If necessary, different Notifiers can/should be instantiated for different recipients. For example, we might define broadcast-ops@, device-ops@, networkblue-ops@, etc. for different operations needs (which could be handled by different teams).

scruzin commented 1 month ago

Implemented in Pull Request #80