-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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: backport guild forum support to v13 #8651
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
What's the breaking change? If there is one, this cannot be merged. |
probably not :c |
Can you be more clear on this? What is the breaking change? A "breaking change" means the modifications will cause old code written for the previous version to not function the same or create errors. Does this pull request have any of that? Please tell us! |
i changed the name of the Class |
name is the same |
yep |
Nice |
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.
As a general review, we should not use resolveAutoArchiveMaxLimit
in new places. We should also tweak the function (in this PR or another one) to always return the maximum value, as they're not longer boost-locked.
I can maybe accept 'MAX'
as a valid option because v13, but it should be discouraged and deprecated (possibly with a deprecation warning). This can be achieved by modifying the function itself, so you don't have to modify multiple files.
However, because that's outside of this PR's scope, I'm approving this anyways. Feel free to make a follow-up PR addressing this review, though.
hmm, do you mean the |
That's right — discord/discord-api-docs#4825 — can't find a blog post or an ddevs announcement for this, but I just tried setting in a test guild of mine (without boosts) to 7 days and it worked just fine. The test guild also doesn't have |
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.
Just one small docs thing 👍🏼
I'm not good at writing docs 😢 |
Last commit looks good, but as I said in the discord.js guild, you need to change the doc because |
so when will it get merged? |
Please describe the changes this PR makes and why it should be merged:
#7791
Status and versioning classification: