-
Notifications
You must be signed in to change notification settings - Fork 69
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
Refactor Senders initialization #804
Conversation
e1153b4
to
d95eeb1
Compare
Codecov Report
@@ Coverage Diff @@
## master #804 +/- ##
==========================================
- Coverage 71.32% 71.30% -0.02%
==========================================
Files 179 179
Lines 9322 9348 +26
==========================================
+ Hits 6649 6666 +17
- Misses 2293 2303 +10
+ Partials 380 379 -1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
b56416b
to
82fdc94
Compare
Want to move senders directory to notifier directory, because senders have no sense without notifier. Also errors.go in the root of Moira contains Notifier erros, should be also moved to notifier dir. |
b62ddd7
to
1fa56b2
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left some comments
notifier/senders/msteams/msteams.go
Outdated
sender.logger = logger | ||
sender.location = location | ||
sender.frontURI = senderSettings["front_uri"] | ||
maxEvents, err := strconv.Atoi(senderSettings["max_events"]) | ||
if err != nil { | ||
return fmt.Errorf("max_events should be an integer: %w", err) | ||
return nil, fmt.Errorf("max_events should be an integer: %w", err) | ||
} | ||
sender.maxEvents = maxEvents | ||
sender.client = &http.Client{ | ||
Timeout: time.Duration(30) * time.Second, //nolint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's move it into const
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, done db17931
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not in func, please :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just drop //nolint
, it is not necessary to refactor all Moira code. This const timeout
is so little thing compare to problems that we have in Moira, so I advise you don't pay attention to it.
ff6e0a5
to
78f40bf
Compare
Use constructor function NewSender instead of Init method, remove Init method from Sender interface.
Rename `init.go` files to `sender.go`, because files no more contain Init method.
Moved senders directory to notifier directory, because senders have no sense without notifier, senders is a part of notifier. Also resolve some warnings
78f40bf
to
8ad4ba1
Compare
merge after release |
and let's make pretty commit messages |
We don't need |
Closes #797