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

Discord notification script #1052

Closed
wants to merge 2 commits into from
Closed

Discord notification script #1052

wants to merge 2 commits into from

Conversation

jasonviviano
Copy link
Contributor

Thank you for helping make the Radius documentation better!

Please follow this checklist before submitting:

  • Read the contribution guide
  • Commands include options for Linux, MacOS, and Windows within codetabs
  • New file and folder names are globally unique
  • Page references use shortcodes instead of markdown or URL links
  • Images use HTML style and have alternative text
  • Places where multiple code/command options are given have codetabs

In addition, please fill out the following to help reviewers understand this pull request:

Description

Explored options for a Discord bot and currently a GitHub Action script seems to fit better in the goal of sending out specific notifications based only on whatever name the current main branch has and notifying users of PR merges.

Right now it looks for a main branch PR merge and sends a message equivalent to the following screenshot to a Discord channel called announcements:

Screenshot 2024-02-21 at 3 08 55 PM

Issue reference

#1000

Signed-off-by: jasonviviano <[email protected]>
Copy link
Contributor

@willtsai willtsai left a comment

Choose a reason for hiding this comment

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

Overall I like this idea, but have a few points of feedback.

Additionally, is there a way to customize which channel in discord these notifications get sent? I'd prefer a dedicated channel for new repo commits instead of bombarding the #announcements channel with this traffic.


on:
push:
branches: [ "main" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

since we don't use a main branch for docs, you might have to figure out how to dynamically fetch the default branch for the docs repo...

Comment on lines +21 to +23
Hi there! The Radius Documentation has been updated. Check out the recent changes:
[View Recent Commits](https://github.com/jasonviviano/project/commits/main/)
[View Recent PRs](https://github.com/jasonviviano/project/pulls)
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a way to enumerate the actual commits in the message itself in addition to these links? this might help users decide whether the changes are interesting enough for them to follow through with a click and seem less "spammy"

@@ -0,0 +1,23 @@
name: Notify Discord on PR Merge
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to notify on every PR merge. Sometimes the PR's are to fix typo's and small errors which might add noise

@kachawla kachawla deleted the jasonviviano-patch-1 branch August 9, 2024 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants