-
Notifications
You must be signed in to change notification settings - Fork 3
Various tweaks #17
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
base: main
Are you sure you want to change the base?
Various tweaks #17
Conversation
Apparently the GitHub Packages Docker Registry is being replaced by the Container registry, which among other advantages allows anonymous access to public images, which seems useful.
When the minimum retention interval supported is an hour, it seems a little silly to check for messages every minute. When the minimum retention interval configured is a month, it seems extra silly. 10% is probably a reasonable error margin in retention interval, and probably means significantly less time processing.
Hey, thanks so much for the PR! 🙌 Definitely going to check it out in the next days. |
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.
Thanks again for putting the time in! Got two tiny notes and it'd be amazing if you could rebase on main to have CI run on this PR.
if: github.ref == 'refs/heads/container' | ||
run: | | ||
docker build -t ghcr.io/${GITHUB_REPOSITORY}/discord-retention-bot:testing . | ||
docker push ghcr.io/${GITHUB_REPOSITORY}/discord-retention-bot:testing |
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.
Can we remove the whole container
branch logic? Does it make sense to build the container on non-main branches but only push it on main
instead?
Discord gateway in order to [send the | ||
messages](https://discord.com/developers/docs/resources/channel#create-message) | ||
that the test deletes (otherwise, you'll see a 40001 Unauthorized JSON error). | ||
You can do this by just making a quick script that calls |
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.
You can do this by just making a quick script that calls | |
You can do this by making a quick script that calls |
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.
Could we have the script somewhere in the repository to make things easier?
I tried setting this up with my own discord server, and made some tweaks in the process. I'm not sure which if any of these would be appealing to you, but just in case.
The changes: