Skip to content

add caption, parse_mode, caption_entities, disable_web_page_preview parameters to Bot._send_message#3385

Merged
Bibo-Joshi merged 7 commits intomasterfrom
caption-params-for-send-message-3372
Nov 26, 2022
Merged

add caption, parse_mode, caption_entities, disable_web_page_preview parameters to Bot._send_message#3385
Bibo-Joshi merged 7 commits intomasterfrom
caption-params-for-send-message-3372

Conversation

@lemontree210
Copy link
Member

@lemontree210 lemontree210 commented Nov 24, 2022 •
edited
Loading

closes #3372

Not sure I need to add tests (I think in this case I just needed to make sure existing tests don't fail).

@lemontree210 lemontree210 added the 🛠 refactor change type: refactor label Nov 24, 2022
@lemontree210 lemontree210 self-assigned this Nov 24, 2022
Bibo-Joshi
Bibo-Joshi requested changes Nov 24, 2022
View reviewed changes
@lemontree210
refactor: add docstring to ._send_message
a2370bc 
closes #3372
add doctstring and move comment there
@lemontree210 lemontree210 added enhancement and removed 🛠 refactor change type: refactor labels Nov 24, 2022
harshil21
harshil21 reviewed Nov 24, 2022
View reviewed changes
Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

copy_message and edit_message_caption still have those if clauses left to remove.

Bibo-Joshi
Bibo-Joshi reviewed Nov 24, 2022
View reviewed changes
@lemontree210
Copy link
Member Author

lemontree210 commented Nov 24, 2022

copy_message and edit_message_caption still have those if clauses left to remove.

Yes, I agree on edit_message_caption(), but copy_message() calls Bot._post(), not Bot._send_message(), so not sure this change is applicable for copy_message()

@lemontree210
Copy link
Member Author

lemontree210 commented Nov 25, 2022

BTW a (maybe too) philosophical question. Can the fact that the generic ._send_message() method knows about other methods having parameters such as .caption (which pertain not to any message but to a specific type of message) be considered as breaking LoD? Maybe it's OK in this context because all of these params are part of official Telegram API.

@Bibo-Joshi
Copy link
Member

Bibo-Joshi commented Nov 25, 2022

fact that the generic ._send_message() method knows about other methods having parameters such as .caption

IMO this is not the case. _send_message just offers that parameter, but it doesn't really know if any other method actually uses it.

@lemontree210
add parse_mode to args of Bot._send_message()
be5cfef 
change all methods except for `.copy_message()`,
which doesn't call `._send_message()`
@lemontree210
Copy link
Member Author

lemontree210 commented Nov 25, 2022

Added parse_mode as mentioned in this comment for #3372

@lemontree210 lemontree210 changed the title refactor: add caption, caption_entities parameters to Bot._send_message refactor: add caption, parse_mode, caption_entities parameters to Bot._send_message Nov 25, 2022
@lemontree210 lemontree210 changed the title refactor: add caption, parse_mode, caption_entities parameters to Bot._send_message add caption, parse_mode, caption_entities parameters to Bot._send_message Nov 25, 2022
Bibo-Joshi
Bibo-Joshi reviewed Nov 25, 2022
View reviewed changes
@lemontree210 lemontree210 changed the title add caption, parse_mode, caption_entities parameters to Bot._send_message add caption, parse_mode, caption_entities, disable_web_page_preview parameters to Bot._send_message Nov 25, 2022
Bibo-Joshi
Bibo-Joshi approved these changes Nov 25, 2022
View reviewed changes
Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

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

LGTM except for the comment below :)

@Bibo-Joshi Bibo-Joshi merged commit 637cc57 into master Nov 26, 2022
@Bibo-Joshi Bibo-Joshi deleted the caption-params-for-send-message-3372 branch November 26, 2022 18:05
@github-actions github-actions bot locked and limited conversation to collaborators Dec 4, 2022
@Bibo-Joshi Bibo-Joshi added 🔌 enhancement pr description: enhancement and removed enhancement labels Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Bibo-Joshi Bibo-Joshi Bibo-Joshi approved these changes

@harshil21 harshil21 harshil21 approved these changes

Assignees

@lemontree210 lemontree210

Labels

🔌 enhancement pr description: enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add caption, caption_entities as parameters to Bot._send_message

3 participants

@lemontree210 @Bibo-Joshi @harshil21