Skip to content

feat: discord notifications #126

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

Merged
merged 4 commits into from
Aug 21, 2024
Merged

feat: discord notifications #126

merged 4 commits into from
Aug 21, 2024

Conversation

nuxencs
Copy link
Owner

@nuxencs nuxencs commented Aug 12, 2024

This implements discord notifications for all the important events. I'm planning to implement more notification services but discord should be enough for now.

Resolves #114

@nuxencs nuxencs added the Type: Enhancement Improvement of the current situation label Aug 12, 2024
@nuxencs nuxencs added this to the v0.9.0 milestone Aug 12, 2024
@nuxencs nuxencs changed the title feat: notifications feat: discord notifications Aug 12, 2024
Copy link

@zze0s zze0s left a comment

Choose a reason for hiding this comment

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

Looks good! Some minor improvements could be made but not required.

I see shoutrrr in the go.mod 😉 I wasted a lot of time going down that road and trying to implement it.

@nuxencs nuxencs requested a review from zze0s August 12, 2024 12:34
@nuxencs nuxencs added the Area: Notifications Related to notifications label Aug 12, 2024
Copy link

@zze0s zze0s left a comment

Choose a reason for hiding this comment

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

Looks good!

Just unsure if DiscordSender should be exported or not. If it runs then it shouldn't be needed 😄

@@ -49,15 +49,15 @@ const (
GRAY EmbedColors = 10070709 // 99aab5
)

type DiscordSender struct {
type discordSender struct {
Copy link

Choose a reason for hiding this comment

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

I think this still needs to be exported no?

Copy link
Owner Author

Choose a reason for hiding this comment

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

it's only used in the notification module so should be fine like this 😄

@nuxencs nuxencs force-pushed the feat/notifications branch from d43893f to 552c85b Compare August 21, 2024 14:52
@nuxencs nuxencs merged commit 6f9b6fd into develop Aug 21, 2024
12 checks passed
@nuxencs nuxencs deleted the feat/notifications branch August 21, 2024 15:07
@bakerboy448
Copy link
Contributor

any control over what is sent or just sends all currently?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Notifications Related to notifications Type: Enhancement Improvement of the current situation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] notifications
3 participants