-
-
Notifications
You must be signed in to change notification settings - Fork 460
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
refactor!: embed classes #2063
refactor!: embed classes #2063
Conversation
we shouldn't set required fields to none. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #2063 +/- ##
==========================================
- Coverage 33.24% 33.22% -0.02%
==========================================
Files 97 97
Lines 19039 19092 +53
==========================================
+ Hits 6329 6343 +14
- Misses 12710 12749 +39
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
nice work om |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All in all a good effort, mainly needs some fine tuning.
Please update all necessary docs;
- attrs like
author
,footer
andvideo
should refer to their new class. - Remove
Empty
andEmbedProxy
references. - Add all new classes such as
EmbedMedia
to data_classes.rst.
Any changes you make to docs can be previewed here.
This pull request is in the In review step of the Default workflow. Since there are no pending review teams, @Om1609 shoulde determine what is required to move this PR forward. |
Not sure if this has been noted as part of the breaking changes but I noticed another breaking change which was the File "bot-dir/venv/lib/python3.11/site-packages/discord/embeds.py", line 995, in to_dict
result["fields"] = [field.to_dict() for field in self._fields]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ |
Yes that seems like a missed bug. I'll fix this in a bit |
Already fixed by #2071 |
Summary
Breaking Change -
Attributes for Author, Footer, Image, Thumbnail, Video, and Provider on the embed will now return
None
instead ofEmbedProxy
, when they are not set. This change is done for correct API reflection.This allowed the complete removal of
EmbedProxy
in favour of separate classes for the above attributes. This allows for concrete typing support. The new classes areEmbedAuthor
,EmbedFooter
,EmbedMedia
andEmbedProvider
Breaking Change -
Embed.Empty
andEmptyEmbed
have now been removed in favour ofNone
Information
examples, ...).
Checklist
type: ignore
comments were used, a comment is also left explaining why.