-
-
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(ClientOptions): waitGuildTimeout amount client option #6576
Conversation
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.
This is also missing documentation and validation in the Client class.
Wouldn't |
I've tried my best to make the recommended changes, let me know if we need to change anything else or we have any other thoughts about this before merging. |
typings: Collector filter parameter inference (discordjs#6574)
Yeah I’m pretty sure that text you added goes over the 120 char limit, you should install ESLint to be aware of these things since the CI requires approval to run |
Co-authored-by: Hackerboi 69 <[email protected]>
You can add the default value for this option in the discord.js/src/util/Options.js Line 110 in 0841956
And add validation in the Client#_validateOptions() method:discord.js/src/client/Client.js Line 542 in 0841956
|
I'm debating if this should be titled "fetchGuildTimeout" or "fetchGuildsTimeout" or something else. I'm happy to discuss this and improve the experience for developers. |
Co-authored-by: monbrey <[email protected]>
Co-authored-by: Hackerboi 69 <[email protected]>
everything in here looks done! let me know if anything else needs to be changed, otherwise, this will be it! happy that this will get merged soon! |
The default value is still not in |
Check the reviews I submitted also. |
dfsdfsdfsdfsdf
merge back
What does.that last commit have to do with this PR? |
It fixes a bug in the discord name acronym system. Apologies.
…On Sat, Nov 27, 2021 at 11:52 PM Hackerboi 69 ***@***.***> wrote:
What does.that last commit have to do with this PR?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6576 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABZQTZVQWAA2UTSZKG33KM3UOHNVHANCNFSM5DFV2GAA>
.
|
I reverted the changes. Really sorry. |
Those changes have been moved to #7050 |
|
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.
The default is still not added in Options.createDefault()
Apologies for not updating this, I will change it. Sorry, i've been busy with finals. |
Again, i'm really sorry about the delay. I added it to options now |
All I want for Christmas..issss... a rebaseeeeee 🎵 |
Please describe the changes this PR makes and why it should be merged:
Sometimes bots hang while connecting for 15 seconds because 1 single guild is messed up. For bots with Continuous integration, this can create significant impacts to uptime, user experience, and lag.
This adds the optional
fetchGuildTimeout
option under ClientOptions, where the bot developer can optionally specify how long the client should hang for. If not specified, this will remain the default 15 seconds.I'm happy to discuss how this can be improved, including the names of the options and variables, and I'd also be happy to write documentation!
👉🏻👈🏻 my first pr to this repo, I'm honoured I can be a part of this community.
EDIT: I'm wondering if the "in X sec" should be changed to "in X ms".
Status and versioning classification: