-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[Utils] [Chat Formatting] New rows to embeds util #4603
base: V3/develop
Are you sure you want to change the base?
Conversation
added rows_to_embeds util (in chat_formatting) added two unit tests
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.
Just some things I've noticed at a glance ... I'm not a fan of the util name but can't think of an alternative either
- Renamed function from `rows_to_embeds` to `embeds_from_rows` - Changed "type, optional" to "Optional[type]". - Add embed field limit check (max = 25).
Updated the files based on the comments above. I renamed the function to |
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.
Alternative naming suggestion: embedify_list
@@ -284,7 +284,7 @@ def pagify( | |||
yield in_text | |||
|
|||
|
|||
def rows_to_embeds( | |||
def embeds_from_rows( |
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.
def embeds_from_rows( | |
def embedify_list( |
Alternative name option. Follows the naming scheme of pagify
@@ -203,7 +203,7 @@ def test_embed_list(): | |||
base_embed = Embed(title="Test") | |||
rows = list(str(i) for i in range(11)) | |||
|
|||
embed_list = chat_formatting.rows_to_embeds( | |||
embed_list = chat_formatting.embeds_from_rows( |
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.
embed_list = chat_formatting.embeds_from_rows( | |
embed_list = chat_formatting.embedify_list( |
@@ -223,7 +223,7 @@ def test_embed_list_2(): | |||
base_embed = Embed(title="Test") | |||
rows = list(str(i) for i in range(11)) | |||
|
|||
embed_list = chat_formatting.rows_to_embeds( | |||
embed_list = chat_formatting.embeds_from_rows( |
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.
embed_list = chat_formatting.embeds_from_rows( | |
embed_list = chat_formatting.embedify_list( |
Type
Description of the changes
rows_to_embeds()
util inchat_formatting
.Resolves #4600.