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

replace DeleteMessageDays with DeleteMessageSeconds and make AddBan take a Duration #195

Merged
merged 2 commits into from Aug 29, 2022

Conversation

mlnrDev
Copy link
Member

@mlnrDev mlnrDev commented Aug 16, 2022

@mlnrDev
Copy link
Member Author

mlnrDev commented Aug 17, 2022

alternatively, we could deprecate DeleteMessageDays and change this method to take discord.AddBan instead of a value and allow the user to choose which field they want to use before delete_message_days gets removed in the next API version. it'd still be breaking, but it'd at least allow the user to take advantage of something which still works while adding support for the new field.

@topi314
Copy link
Member

topi314 commented Aug 17, 2022

hm this one is quite annoying as it doesn't break the method signature. What if we instead make the method take time.Duration?
sadly this will still not break the signature and could lead to confusion. On the flip side I don't think a lot of people use(if any) so if we break it now it should be fine

@mlnrDev mlnrDev changed the title replace DeleteMessageDays with DeleteMessageSeconds replace DeleteMessageDays with DeleteMessageSeconds and make AddBan take a Duration Aug 17, 2022
@mlnrDev mlnrDev marked this pull request as ready for review August 29, 2022 18:57
@mlnrDev mlnrDev merged commit 606be43 into development Aug 29, 2022
@mlnrDev mlnrDev deleted the patch/replace-delete-message-days branch August 29, 2022 19:18
@mlnrDev mlnrDev added the t:rest label Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants