Skip to content

feat: add new parameters to discord.Embed#1996

Merged
Lulalaby merged 13 commits intoPycord-Development:masterfrom
davidhozic:feat/embed_parameters
Apr 18, 2023
Merged

feat: add new parameters to discord.Embed#1996
Lulalaby merged 13 commits intoPycord-Development:masterfrom
davidhozic:feat/embed_parameters

Conversation

@davidhozic
Copy link
Contributor

@davidhozic davidhozic commented Apr 2, 2023 •
edited
Loading

Summary

This PR adds parameters to discord.Embed as proposed in #1995.
It also adds 2 new classes to support the latter. The classes are: EmbedFooter and EmbedAuthor

Example:

import discord
import secret


embed = discord.Embed(
    author=discord.EmbedAuthor(
        name="Mr. President",
        icon_url="https://media.discordapp.net/attachments/1044657935608987808/1065995582654664724/svetlogo.png"
    ),
    footer=discord.EmbedFooter(text="This is a footer")
)


client = discord.Client()


@client.listen("on_ready")
async def start():
    channel = client.get_channel(1091835594243592293)
    await channel.send(embed=embed)
    await client.close()


client.run(secret.TOKEN)

which produces

Information

  • 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, typehinting,
    examples, ...).

Checklist

  • I have searched the open pull requests for duplicates.
  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why.
  • I have updated the changelog to include these changes.

@davidhozic davidhozic requested a review from a team as a code owner April 2, 2023 11:45
@davidhozic davidhozic changed the title Embed - add parameters feat:Embed - add parameters Apr 2, 2023
@davidhozic davidhozic changed the title feat:Embed - add parameters feat: add new parameters to discord.Embed Apr 2, 2023
@codecov
Copy link

codecov bot commented Apr 2, 2023 •
edited
Loading

Codecov Report

Merging #1996 (9dc4548) into master (8fb956c) will decrease coverage by 0.01%.
The diff coverage is 28.57%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1996      +/-   ##
==========================================
- Coverage   33.33%   33.32%   -0.01%     
==========================================
  Files          97       97              
  Lines       18900    18916      +16     
==========================================
+ Hits         6300     6304       +4     
- Misses      12600    12612      +12
Flag Coverage Δ
macos-latest-3.10 33.31% <28.57%> (-0.01%) ⬇️
macos-latest-3.11 33.31% <28.57%> (-0.01%) ⬇️
macos-latest-3.8 33.32% <28.57%> (-0.01%) ⬇️
macos-latest-3.9 33.32% <28.57%> (-0.01%) ⬇️
ubuntu-latest-3.10 33.31% <28.57%> (-0.01%) ⬇️
ubuntu-latest-3.11 33.31% <28.57%> (-0.01%) ⬇️
ubuntu-latest-3.8 33.32% <28.57%> (-0.01%) ⬇️
ubuntu-latest-3.9 33.32% <28.57%> (-0.01%) ⬇️
windows-latest-3.10 33.31% <28.57%> (-0.01%) ⬇️
windows-latest-3.11 33.31% <28.57%> (-0.01%) ⬇️
windows-latest-3.8 33.32% <28.57%> (-0.01%) ⬇️
windows-latest-3.9 33.32% <28.57%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
discord/embeds.py 25.00% <28.57%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fb956c...9dc4548. Read the comment docs.

@Lulalaby Lulalaby requested a review from JustaSqu1d April 2, 2023 12:25
JustaSqu1d
JustaSqu1d requested changes Apr 2, 2023
View reviewed changes
Copy link
Member

@JustaSqu1d JustaSqu1d left a comment

Choose a reason for hiding this comment

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

Also, note the change in their respective properties.

@JustaSqu1d
Copy link
Member

JustaSqu1d commented Apr 2, 2023

Should we deprecate set_author, etc?

@JustaSqu1d JustaSqu1d added priority: medium Medium Priority status: awaiting review Awaiting review from a maintainer feature Implements a feature python labels Apr 2, 2023
@davidhozic
Copy link
Contributor Author

davidhozic commented Apr 2, 2023

Should we deprecate set_author, etc?

I think that's a good idea yes

@davidhozic
Copy link
Contributor Author

davidhozic commented Apr 2, 2023

Should we deprecate set_author, etc?

I think that's a good idea yes

On second thought, I'd leave it in, you still have the set_field_at and remove_field and other remove_ methods available.
Maybe you for example want to modify an embed from a received message and send it back, so these methods can be helpful.

@Lulalaby
Copy link
Member

Lulalaby commented Apr 2, 2023

Should we deprecate set_author, etc?

No, not in every case you want to use the constructor.
And what if you want to modify it afterwards

JustaSqu1d
JustaSqu1d previously approved these changes Apr 2, 2023
View reviewed changes
@davidhozic davidhozic mentioned this pull request Apr 2, 2023
Closed
@Lulalaby Lulalaby linked an issue Apr 2, 2023 that may be closed by this pull request
Add missing parameters to Embed #1995
Closed
Lulalaby
Lulalaby previously approved these changes Apr 2, 2023
View reviewed changes
@Lulalaby Lulalaby enabled auto-merge (squash) April 2, 2023 16:26
@Nzii3
Copy link
Contributor

Nzii3 commented Apr 2, 2023

I mean, I don't really see the point in this but, idk.

Nzii3
Nzii3 previously approved these changes Apr 2, 2023
View reviewed changes
plun1331
plun1331 reviewed Apr 2, 2023
View reviewed changes
Copy link
Member

@plun1331 plun1331 left a comment

Choose a reason for hiding this comment

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

Ideally, embed.footer and embed.author should return their respective classes

auto-merge was automatically disabled April 2, 2023 20:31

Head branch was pushed to by a user without write access

@davidhozic davidhozic dismissed stale reviews from Nzii3, Lulalaby, and JustaSqu1d via 65417f1 April 2, 2023 20:31
@davidhozic
Copy link
Contributor Author

davidhozic commented Apr 2, 2023 •
edited
Loading

Not sure what proxy_icon_url is supposed to be, but I'll leave it in the constructor since it's needed for the @properties and it will give you an error anyways if you try to provide it manually since Embed internally calls the set_ methods in it's constructor.

@plun1331
Copy link
Member

plun1331 commented Apr 3, 2023

Not sure what proxy_icon_url is supposed to be, but I'll leave it in the constructor since it's needed for the @properties and it will give you an error anyways if you try to provide it manually since Embed internally calls the set_ methods in it's constructor.

it's a value returned by discord that proxies the image through their cdn iirc

@Lulalaby
Copy link
Member

Lulalaby commented Apr 3, 2023

it's a value returned by discord that proxies the image through their cdn iirc

That's correct

@Lulalaby Lulalaby merged commit f553fde into Pycord-Development:master Apr 18, 2023
@tibue99 tibue99 mentioned this pull request May 2, 2023
Closed
@Lulalaby Lulalaby mentioned this pull request Aug 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lulalaby Lulalaby Lulalaby approved these changes

@JustaSqu1d JustaSqu1d JustaSqu1d approved these changes

@VincentRPS VincentRPS Awaiting requested review from VincentRPS

@Nzii3 Nzii3 Awaiting requested review from Nzii3

@plun1331 plun1331 Awaiting requested review from plun1331

Assignees

No one assigned

Labels

feature Implements a feature priority: medium Medium Priority status: awaiting review Awaiting review from a maintainer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add missing parameters to Embed

5 participants

@davidhozic @JustaSqu1d @Lulalaby @Nzii3 @plun1331