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

Adding two examples #265

Merged
merged 7 commits into from Oct 6, 2019

Conversation

@Darqam
Copy link
Contributor

commented Oct 5, 2019

Adding an example for fetching (counting) all online members in a guild as well as a way to monitor role addition and removal in the guildMemberAdd event.

Prompted by the ToDo section(s) by Souji (so blame him if it's unwanted).

Darqam added 3 commits Oct 5, 2019
Copy link

left a comment

Requires some simplicity, grammar enhancements, and lastly, code style.
Furthermore, the question is more of a miscellaneous question than an administrative one.

@GamesProSeif

This comment has been minimized.

Copy link

commented Oct 5, 2019

I agree with you in the former, code style needs some changes.
Though I feel these changes fall correctly under the common-questions category, since they're part of FAQs.
Souji did state the category (FAQ) in the project Page ideas.

guide/popular-topics/common-questions.md Outdated Show resolved Hide resolved
guide/popular-topics/common-questions.md Outdated Show resolved Hide resolved
guide/popular-topics/common-questions.md Outdated Show resolved Hide resolved
guide/popular-topics/common-questions.md Outdated Show resolved Hide resolved
@Darqam

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2019

I believe this addresses the concerns.

With respect to the comments about code style, I am not quite sure what the two of you mean. some and filter are both methods inherited/modified from arrays, so there's nothing that should be black magic to users.

The code could be far more explicit with for loops or something, but that would seem kind of pointless to me given the provided tools. I could very well be missing a way to make this simpler or cleaner, if so feel free to mention it, however as it stands I don't see it.

I'll also note that now the console.log returns assuming plurality of roles changed (which I forgot because setRoles is a thing), and will stay as such for single role changes. One could make a size check and adjust the sentence accordingly, but this seems like more noise than is necessary for such an example.

Copy link
Member

left a comment

Logs are okay, plurality is also fine.
What i don't like is the filtering by name rather than id, as roles can have the same name.
(With the added benefit of has(role.id) being more straight-forward to read than role and roleTwo)

Thanks for addressing my earlier concerns so quickly 👍

guide/popular-topics/common-questions.md Outdated Show resolved Hide resolved
guide/popular-topics/common-questions.md Outdated Show resolved Hide resolved
@Darqam

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2019

Right, id makes far more sense and is also much simpler to read.

guide/popular-topics/common-questions.md Outdated Show resolved Hide resolved
Co-Authored-By: SpaceEEC <spaceeec@yahoo.com>
@almostSouji almostSouji merged commit 0a39196 into discordjs:master Oct 6, 2019
3 of 6 checks passed
3 of 6 checks passed
Header rules No header rules processed
Details
Pages changed 49 new files uploaded
Details
Redirect rules No redirect rules processed
Details
Mixed content No mixed content detected
Details
ci/circleci Your tests passed on CircleCI!
Details
deploy/netlify Deploy preview ready!
Details
Darqam added a commit to Darqam/guide that referenced this pull request Oct 6, 2019
Per discordjs#265

Note changes on fetchMembers() -> members.fetch()
and return guild -> return members collection
almostSouji added a commit that referenced this pull request Oct 6, 2019
* online member count
* identify removed and added roles on guildMemberUpdate
* v12 adaptation of #265
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.