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

feat: new notification provider 46elks #5184

Merged
merged 31 commits into from
Oct 11, 2024
Merged

Conversation

erlaan
Copy link
Contributor

@erlaan erlaan commented Oct 10, 2024

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

  • I have read and understand the pull request rules.

Description

This PR adds 46elks as a notification channel for Uptime Kuma

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas (including JSDoc for methods)
  • My changes generates no new warnings

Screenshots (if any)

Event Before After
UP - 2024-10-10-17-16-58
DOWN - 2024-10-10-17-16-30
Testing - 2024-10-10-17-14-27
Testing - Screenshot_20241010-171037

Please do not use any external image service. Instead, just paste in or drag and drop the image here, and it will be uploaded automatically.

Copy link
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

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

Thanks for the PR ♥️
I have left a few suggestions below how this PR can be improved in terms of the UI and code quality.

server/notification-providers/46elks.js Outdated Show resolved Hide resolved
server/notification-providers/46elks.js Outdated Show resolved Hide resolved
src/components/notifications/46elks.vue Outdated Show resolved Hide resolved
src/lang/en.json Outdated Show resolved Hide resolved
src/components/notifications/46elks.vue Show resolved Hide resolved
src/components/notifications/46elks.vue Outdated Show resolved Hide resolved
src/components/notifications/46elks.vue Outdated Show resolved Hide resolved
server/notification-providers/46elks.js Outdated Show resolved Hide resolved
@CommanderStorm CommanderStorm changed the title New Notification Provbider 46elks feat: new notification provider ´46elks´ Oct 10, 2024
@CommanderStorm CommanderStorm changed the title feat: new notification provider ´46elks´ feat: new notification provider 46elks Oct 10, 2024
@CommanderStorm CommanderStorm added area:notifications Everything related to notifications type:new proposing to add a new monitor pr:please address review comments this PR needs a bit more work to be mergable labels Oct 10, 2024
@erlaan
Copy link
Contributor Author

erlaan commented Oct 11, 2024

Thanks for the PR ♥️ I have left a few suggestions below how this PR can be improved in terms of the UI and code quality.

Thx for the input I think i have fixed all the suggestion now. :)

Copy link
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

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

Thanks for the new notification provider! 🎉

Note

This PR is part of the v2.0 merge window => see #4500 for the bugs that need to be addressed before we can ship this release ^^

All changes in this PR are small and uncontroversial ⇒ merging with junior maintainer approval

src/components/notifications/46elks.vue Outdated Show resolved Hide resolved
src/lang/en.json Outdated Show resolved Hide resolved
server/notification-providers/46elks.js Outdated Show resolved Hide resolved
@CommanderStorm CommanderStorm merged commit dda4061 into louislam:master Oct 11, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:notifications Everything related to notifications pr:please address review comments this PR needs a bit more work to be mergable type:new proposing to add a new monitor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants