Skip to content

feat: auto-delete in voice-chat feature#72

Merged
SCDerox merged 32 commits intoScootKit:mainfrom
torbenprobst:auto-delete-inVoiceFeature
Sep 9, 2022
Merged

feat: auto-delete in voice-chat feature#72
SCDerox merged 32 commits intoScootKit:mainfrom
torbenprobst:auto-delete-inVoiceFeature

Conversation

@torbenprobst
Copy link
Contributor

This is the feature PR for the module auto-delete.

Features added:

  • Auto deleting messages after a certain time if the last member of a voice channel left.
  • PurgeOnStart on in-Voice Channels only if the channel is empty
  • finding and deleting of duplicate entries in channel/voice-channels config
  • On duplicate entries in channel and voice-Channels use entry of voice-channel config

Tests:

  • i´ve tried to test every scenario on a real discord server

Copy link
Member

@SCDerox SCDerox left a comment

Choose a reason for hiding this comment

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

Hi, thanks so much for your contribution!
I found have some small things which I have added as comments. The features of the changes seem to work correctly (tested in prod-like environment). Also: Do not feel pressured to implement my comments right away, I am happy to discuss them if you feel like your way of doing something is better (except the missing import, that should be fixed).
Thanks again for your contribution!

If you want to receive a small bountie for this contribution, please follow these instructions before re-requesting a review. Thanks ^^

@torbenprobst torbenprobst force-pushed the auto-delete-inVoiceFeature branch from 85a13b9 to 1bb6a32 Compare August 5, 2022 14:51
@torbenprobst
Copy link
Contributor Author

Would be good if you double check the changes again. I did some refactoring and added error handling for the "channel.message.fetch"

@torbenprobst torbenprobst requested a review from SCDerox August 5, 2022 14:59
Copy link
Member

@SCDerox SCDerox left a comment

Choose a reason for hiding this comment

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

Thanks so much for your contribution! Unfortunatly, as of right now, the feature you want to add does not work properly. Please fix the mentioned bug in my comments.

@SCDerox
Copy link
Member

SCDerox commented Aug 11, 2022

Oh also btw: We tend to use a different commit-message-format (compare https://github.com/SCNetwork/CustomDCBot/commits/main), it would be great if you could adapt it for future commits.

@torbenprobst
Copy link
Contributor Author

I have added some more detailed description in both languages. Does it work?
2d50b73

Couldn´t find any info about that in the README.md

@torbenprobst torbenprobst requested a review from SCDerox August 12, 2022 11:23
Copy link
Member

@SCDerox SCDerox left a comment

Choose a reason for hiding this comment

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

Sorry for annoying you once again, could you upgrade your changes to v3.5.0? Should only affect locales as far as I can see. (also everything works now with your changes; i have found two localization issues, please fix them in both files)

@torbenprobst torbenprobst requested a review from SCDerox August 12, 2022 23:27
@torbenprobst
Copy link
Contributor Author

I will upgrade it to the latest version as soon as i have time. I didnt read your message

@torbenprobst
Copy link
Contributor Author

I updated the branch to the latest custombot version. You have to solve a little Merge Conflict because i had to add a new locale entry. Look there ->65bcb4f

Check @SCDerox

@SCDerox
Copy link
Member

SCDerox commented Aug 13, 2022

Sorry, but I can not fix these merge issues as I am not the owner of the torbenprobst/CustomDCBot repository. Please try fixing the merge conflict yourself, feel free to reach out if you need help.

@torbenprobst torbenprobst force-pushed the auto-delete-inVoiceFeature branch from 2e2756b to 563f64c Compare September 8, 2022 14:19
@torbenprobst
Copy link
Contributor Author

torbenprobst commented Sep 8, 2022

Should be fixed now. Pls review again @SCDerox

Copy link
Member

@SCDerox SCDerox left a comment

Choose a reason for hiding this comment

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

lgtm 👍

Thanks so much for your contribution and sorry for all the pity changes I requested. Your code will be merged shortly. SCNX users can expect to receive your update on the beta branch this afternoon. Unfortunately, we can not share an ETA for the main branch, but it's typically 1-2 work days.

You'll receive small bounty in accordance with our Open-Source-Developer-Program credited to you organization. If you have any questions about this or the payout of the amount, please contact ScootKit support.

Thanks again!

@SCDerox SCDerox merged commit b93efd6 into ScootKit:main Sep 9, 2022
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.

2 participants