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

Implement user banner & banner color #1736

Merged
merged 25 commits into from
Nov 7, 2021
Merged

Implement user banner & banner color #1736

merged 25 commits into from
Nov 7, 2021

Conversation

RedDaedalus
Copy link
Contributor

@RedDaedalus RedDaedalus commented Jul 19, 2021

Pull Request Etiquette

  • I have checked the PRs for upcoming features/bug fixes.
  • I have read the [contributing guidelines][contributing].

Changes

  • Internal code
  • Library interface (affecting end-user code)
  • Documentation

Closes Issue: NaN

Description

Implements basic support for user banners. This is not very usable currently, missing docs, and is expected to break when the Discord API is changed shortly.
Docs PR: discord/discord-api-docs#3448

Considerations

• Banner information is only sent when the user is specifically requested, a method to retrieve this might be beneficial.
• The handling of unknown state from passively gathered user objects
• The changes that will be made to the banner_color field are currently not known in detail.

@DV8FromTheWorld DV8FromTheWorld added the status: freezer this is currently put on hold label Aug 2, 2021
@DV8FromTheWorld
Copy link
Member

Placed in freezer until Docs PR is merged

@MinnDevelopment MinnDevelopment removed the status: freezer this is currently put on hold label Aug 16, 2021
@RedDaedalus RedDaedalus marked this pull request as ready for review August 29, 2021 05:26
@RedDaedalus
Copy link
Contributor Author

The docs PR has been merged.

@RedDaedalus RedDaedalus changed the title Initial pass on user banners Implement user banner & banner color Aug 29, 2021
RedDaedalus and others added 3 commits September 4, 2021 18:07
• Adds `getAccentColorRaw` method for the accent color.
• Adds missing copyright to events.
• Clarifies some docs and adds missing annotations.
@DV8FromTheWorld
Copy link
Member

All of these new properties are related to a user's profile.
I feel like we shouldn't add all of these top-level getters and should instead just have the POJO for Profile

@RedDaedalus
Copy link
Contributor Author

Just went through reviews, apologies for the delay here.
The way I've implemented this, retrieveProfile returns a cached profile only when using a User directly produced by retrieving them. This prevents redundant getters, but also feels a bit confusing to me. Not sure if there's a better approach however.

RedDaedalus and others added 7 commits November 5, 2021 22:21
Co-authored-by: Florian Spieß <business@minn.dev>
…java

Co-authored-by: Florian Spieß <business@minn.dev>
Co-authored-by: Florian Spieß <business@minn.dev>
Co-authored-by: Florian Spieß <business@minn.dev>
Co-authored-by: Florian Spieß <business@minn.dev>
…java

Co-authored-by: Florian Spieß <business@minn.dev>
…java

Co-authored-by: Florian Spieß <business@minn.dev>
@RedDaedalus
Copy link
Contributor Author

All review suggestions have been committed.

@Chew
Copy link
Contributor

Chew commented Nov 6, 2021

For the future, you can batch apply suggestions for code review...

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.

None yet

5 participants