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

feat(GuildPreview): implement support for "preview" endpoint #3965

Merged
merged 34 commits into from Mar 27, 2020

Conversation

@SouSinner
Copy link
Contributor

SouSinner commented Mar 19, 2020

Please describe the changes this PR makes and why it should be merged:
This PR implements a new feature supporting the preview endpoint which is available for public guilds. As mentioned in #3958 — the PR includes the following changes:

  • fetchGuildPreview() — method on an instance of Client, used to fetch a public guild if available
  • GuildPreview — a structure for an instance of a Guild that's been fetched through the preview endpoint

If anything else needs updating, or if there's been a mistake anywhere, please don't hesitate to point it out; I'll try to resolve it immediately. Also, I'd like to apologize for the former PR that I've requested on my main account, I accidentally pushed changes from my original branch.

Status

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating

Semantic versioning classification:

  • This PR changes the library's interface (methods or parameters added)
    • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
  • This PR only includes non-code changes, like changes to documentation, README, etc.
@SouSinner SouSinner changed the title feature: implement support for `preview` endpoint feature: implement support for "preview" endpoint Mar 19, 2020
src/client/Client.js Outdated Show resolved Hide resolved
src/client/Client.js Outdated Show resolved Hide resolved
src/structures/GuildPreview.js Outdated Show resolved Hide resolved
Copy link

Sardonyx78 left a comment

Can be more documented

@Sardonyx78

This comment was marked as off-topic.

Copy link

Sardonyx78 commented Mar 19, 2020

Can be more documented

I didnt meant it can be less documented ...

@SouSinner

This comment was marked as off-topic.

Copy link
Contributor Author

SouSinner commented Mar 19, 2020

Can be more documented

I didnt meant it can be less documented ...

Maybe I'm just dumb, but I don't think I get what you mean exactly. Is there some part of this PR that doesn't document the methods, properties, and such, correctly? Or that it's missing parts which should be documented?

@Sardonyx78

This comment was marked as off-topic.

Copy link

Sardonyx78 commented Mar 19, 2020

Can be more documented

I didnt meant it can be less documented ...

Maybe I'm just dumb, but I don't think I get what you mean exactly. Is there some part of this PR that doesn't document the methods, properties, and such, correctly? Or that it's missing parts which should be documented?

Sugden meant

Is there some part of this PR that doesn't document the methods, properties, and such, correctly?

I meant

that it's missing parts which should be documented

@SouSinner

This comment has been minimized.

Copy link
Contributor Author

SouSinner commented Mar 19, 2020

I meant
that it's missing parts which should be documented

Could you point the lines of code you believe aren't documented properly? I'll get on it immediately.

typings/index.d.ts Outdated Show resolved Hide resolved
Copy link
Member

SpaceEEC left a comment

Looking good so far, just a few things:

src/structures/GuildPreview.js Show resolved Hide resolved
src/client/Client.js Outdated Show resolved Hide resolved
src/structures/GuildPreview.js Outdated Show resolved Hide resolved
src/structures/GuildPreview.js Outdated Show resolved Hide resolved
src/client/Client.js Show resolved Hide resolved
src/structures/GuildPreview.js Outdated Show resolved Hide resolved
src/structures/GuildPreview.js Outdated Show resolved Hide resolved
src/structures/GuildPreview.js Outdated Show resolved Hide resolved
src/structures/GuildPreview.js Outdated Show resolved Hide resolved
src/client/Client.js Outdated Show resolved Hide resolved
@SpaceEEC SpaceEEC linked an issue that may be closed by this pull request Mar 19, 2020
@SketchyLxve

This comment has been minimized.

Copy link
Contributor

SketchyLxve commented Mar 19, 2020

I've pushed the new changes, hopefully they meet the requirements of what you described, and the methods (and typings) implemented/changed are good now.
However, @SpaceEEC, as you know, I am still conflicted on how to go about the GuildPreview#emojis property. Building a new manager for it seems redundant and unnecessary, however the utility is something we'd like to keep.
Let me know if the way the property is being populated is good, or if there's another way we should go about this. 👍

@advaith1

This comment has been minimized.

Copy link
Contributor

advaith1 commented Mar 19, 2020

Maybe we could also add a fetchPreview() method on Guild as I suggested, for ease of use? like how we have GuildMember.ban() and Guild.members.ban(member)

@SketchyLxve

This comment has been minimized.

Copy link
Contributor

SketchyLxve commented Mar 19, 2020

Sounds good. I'll get on it right now. 👍

typings/index.d.ts Outdated Show resolved Hide resolved
src/structures/GuildPreview.js Outdated Show resolved Hide resolved
typings/index.d.ts Outdated Show resolved Hide resolved
typings/index.d.ts Outdated Show resolved Hide resolved
typings/index.d.ts Outdated Show resolved Hide resolved
typings/index.d.ts Outdated Show resolved Hide resolved
typings/index.d.ts Outdated Show resolved Hide resolved
typings/index.d.ts Outdated Show resolved Hide resolved
src/structures/BaseEmoji.js Outdated Show resolved Hide resolved
src/structures/BaseEmoji.js Outdated Show resolved Hide resolved
typings/index.d.ts Outdated Show resolved Hide resolved
src/structures/BaseEmoji.js Outdated Show resolved Hide resolved
SouSinner and others added 8 commits Mar 22, 2020
@iCrawl iCrawl changed the title feature: implement support for "preview" endpoint feat(GuildPreview): implement support for "preview" endpoint Mar 27, 2020
@iCrawl iCrawl merged commit 88133d0 into discordjs:master Mar 27, 2020
3 checks passed
3 checks passed
ESLint
Details
TSLint
Details
Documentation
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

8 participants
You can’t perform that action at this time.