Skip to content

Conversation

@shiftinv
Copy link
Member

@shiftinv shiftinv commented Aug 10, 2022

Summary

Refactors application models, deduplicating and generally merging related code.

The diff is fairly unreadable as-is, skipping the first commit (that just moves PartialAppInfo above AppInfo) improves readability quite a bit.

tl;dr on the important changes, see changelog for everything:

  • new hierarchy: PartialAppInfo < AppInfo < BotAppInfo (where BotAppInfo is equivalent to the old AppInfo)
    • note: this shouldn't have any practical effect on user code unless these types are created there
  • AppInfo.summary is no more (deprecated in previous version)
  • Invite.target_application (which is now AppInfo) contains more fields
  • now using the correct cover_image url format

Checklist

  • If code changes were made, then they have been tested
    • I have updated the documentation to reflect the changes
    • I have formatted the code properly by running task lint
    • I have type-checked the code by running task pyright
  • This PR fixes an issue
  • This PR adds something new (e.g. new method or parameters)
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

@shiftinv shiftinv added t: enhancement New feature breaking change Includes breaking changes to code/packaging p: medium Medium priority s: needs review Issue/PR is awaiting reviews t: bugfix labels Aug 10, 2022
@shiftinv shiftinv added this to the disnake v2.6 milestone Aug 10, 2022
@shiftinv shiftinv changed the title refactor: application models refactor(appinfo)!: application models Aug 19, 2022
return Widget(state=self._connection, data=data)

async def application_info(self) -> AppInfo:
async def application_info(self) -> BotAppInfo:
Copy link
Member

Choose a reason for hiding this comment

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

Might it be worth caching this information? IIRC we receive the full app info on on_ready/identify when we receive the flags of the application. If not, it may be worth caching this information anyways (due to the ext.commands extension using it anyways, and this information being quite useful regardless)

Copy link
Member Author

Choose a reason for hiding this comment

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

Caching this should be up to users, since the data could change at any time - ext.commands already caches the owner/owners, which is the only part it currently needs, and the ready event only contains app ID + flags :/

@shiftinv shiftinv changed the title refactor(appinfo)!: application models refactor(appinfo)!: improve application models Aug 23, 2022
@shiftinv shiftinv removed this from the disnake v2.6 milestone Sep 10, 2022
@onerandomusername onerandomusername force-pushed the refactor/appinfo branch 2 times, most recently from d16e21f to 53aaf1b Compare October 1, 2022 18:29
@onerandomusername
Copy link
Member

@shiftinv would you please resolve conflicts?

@shiftinv
Copy link
Member Author

would you please resolve conflicts?

@onerandomusername done.

@shiftinv shiftinv removed this from the disnake v2.8 milestone Feb 5, 2023
@onerandomusername
Copy link
Member

@shiftinv would you please resolve conflicts again?

@shiftinv shiftinv added the 3.0 label Feb 24, 2024
@onerandomusername
Copy link
Member

@shiftinv Would you please resolve conflicts?

@onerandomusername
Copy link
Member

I promise I'll review it this time.

@onerandomusername onerandomusername requested review from onerandomusername and removed request for onerandomusername September 20, 2025 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.0 breaking change Includes breaking changes to code/packaging p: medium Medium priority s: needs review Issue/PR is awaiting reviews t: bugfix t: enhancement New feature

Development

Successfully merging this pull request may close these issues.

2 participants