Skip to content

Add Icon to DiscordInviteGuild#582

Merged
Emzi0767 merged 1 commit into
DSharpPlus:masterfrom
js6pak:DiscordInviteGuild-Icon
May 29, 2020
Merged

Add Icon to DiscordInviteGuild#582
Emzi0767 merged 1 commit into
DSharpPlus:masterfrom
js6pak:DiscordInviteGuild-Icon

Conversation

@js6pak

@js6pak js6pak commented May 22, 2020

Copy link
Copy Markdown
Contributor

Summary

Adds IconHash and IconUrl to DiscordInviteGuild

Notes

I have one question, wouldn't it be easier to make DiscordInviteGuild and DiscordGuild share code via inheritance?

@sh30801

sh30801 commented May 24, 2020

Copy link
Copy Markdown
Contributor

Overall this looks good to me.

I have one question, wouldn't it be easier to make DiscordInviteGuild and DiscordGuild share code via inheritance?

This would be preferred but I think it would make the naming conventions somewhat confusing, such as having DiscordGuild inherit from DiscordInviteGuild, which doesn't really make sense. If you were to try and rename it to BaseDiscordGuild, that wouldn't really make sense for it to be a property of an invite. Finally, it could be replaced with just a DiscordGuild with a lot of properties being null, but then again that is an issue throughout the library.

I actually did something similar here, however I don't know if this is a good idea to keep implementing.

@js6pak

js6pak commented May 24, 2020

Copy link
Copy Markdown
Contributor Author

If it was named DiscordPartialGuild, it would make sense for me.

@sh30801

sh30801 commented May 24, 2020

Copy link
Copy Markdown
Contributor

If it was named DiscordPartialGuild, it would make sense for me.

Indeed, I would say that's a good idea then unless someone says otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants