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

Link to ShardMessenger::chunk_guilds from the Guild.members field #683

Merged
merged 2 commits into from
Aug 15, 2019
Merged

Link to ShardMessenger::chunk_guilds from the Guild.members field #683

merged 2 commits into from
Aug 15, 2019

Conversation

Alch-Emi
Copy link
Contributor

The Guild members field details that not all users might be available, but doesn't specify that how one might request that the users be made available. In addition, the method that one would use to accomplish this is kinda hard to find, being on a struct that you can write an entire program without touching once. Linking to that method from the members field might clarify this a lot. Sorry for the small PR, but I think this might be an important change (that would certainly have saved me a ton of time and confusion)

@arqunis arqunis added docs Related to documentation. enhancement An improvement to Serenity. gateway Related to the `gateway` module. model Related to the `model` module. labels Aug 11, 2019
@Lakelezz Lakelezz self-requested a review August 11, 2019 16:38
Copy link
Contributor

@Lakelezz Lakelezz left a comment

Choose a reason for hiding this comment

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

Hello, thanks for your pull request :)

I personally would advice against explaining the function in the first place. The link should suffice.

My reasoning: If the function ever changes, the user will always have the updated information when clicking on the link. Furthermore, the function's documentation must be good enough to stand on its own ground.

Therefore, explaining that this functions allows to manually request remaining guild members, should be sufficient.

@Alch-Emi
Copy link
Contributor Author

Alch-Emi commented Aug 14, 2019

Makes sense! I tend to get a bit overexplainy. How does this look? Should it maybe be reduced to just a **See Also**?

@Lakelezz Lakelezz changed the title Link to ShardMessenger::chunk_guilds from the Guild.members feild Link to ShardMessenger::chunk_guilds from the Guild.members field Aug 15, 2019
@Lakelezz
Copy link
Contributor

It's fine, thanks : )

@Lakelezz Lakelezz merged commit 8e926f9 into serenity-rs:current Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Related to documentation. enhancement An improvement to Serenity. gateway Related to the `gateway` module. model Related to the `model` module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants