Skip to content

Added methods to quickly reply to a message with Markdown or HTML - #827

Merged
tsnoam merged 14 commits into
masterfrom
reply-text-markdown-2
Feb 12, 2018
Merged

Added methods to quickly reply to a message with Markdown or HTML#827
tsnoam merged 14 commits into
masterfrom
reply-text-markdown-2

Conversation

@JosXa

@JosXa JosXa commented Sep 11, 2017

Copy link
Copy Markdown
Contributor

I had problems mergin urllib3, so this PR closes #725.

@Eldinnie Indentation adjusted as per your request.

Tests are probably gonna have to wait until pytest stops messing up my workflow...

@codecov

codecov Bot commented Sep 14, 2017

Copy link
Copy Markdown

Codecov Report

Merging #827 into master will decrease coverage by 0.15%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #827      +/-   ##
==========================================
- Coverage   91.83%   91.67%   -0.16%     
==========================================
  Files         103      103              
  Lines        4052     4061       +9     
  Branches      639      639              
==========================================
+ Hits         3721     3723       +2     
- Misses        193      197       +4     
- Partials      138      141       +3
Impacted Files Coverage Δ
telegram/message.py 96.36% <100%> (-0.63%) ⬇️
telegram/utils/request.py 67.85% <0%> (-0.9%) ⬇️
telegram/bot.py 87.51% <0%> (-0.5%) ⬇️

@Eldinnie

Copy link
Copy Markdown
Member

I edited the docstring and merged master. But it's still missing tests for these new methods.
Please add using monkeypatch. See test_message::test_reply_text for example

@Eldinnie Eldinnie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But it's still missing tests for these new methods.
Please add using monkeypatch. See test_message::test_reply_text for example

@Eldinnie

Copy link
Copy Markdown
Member

@JosXa what is the status for this?

@JosXa

JosXa commented Oct 2, 2017

Copy link
Copy Markdown
Contributor Author

Tests added as discussed

@JosXa

JosXa commented Oct 2, 2017

Copy link
Copy Markdown
Contributor Author

Sorry, I keep committing files from other branches... Fixed now.

@JosXa JosXa self-assigned this Oct 7, 2017

@Eldinnie Eldinnie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some small docstring improvements and please restore the dict in test_message to the state it was.

Other than that it looks good to me.

Comment thread telegram/message.py Outdated
def reply_markdown(self, *args, **kwargs):
"""Shortcut for::

bot.sendMessage(update.message.chat_id, parse_mode=ParseMode.MARKDOWN, *args, **kwargs)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please use the snake_case names in docs. (send_message)

Comment thread telegram/message.py Outdated
def reply_html(self, *args, **kwargs):
"""Shortcut for::

bot.sendMessage(update.message.chat_id, parse_mode=ParseMode.HTML, *args, **kwargs)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same as above

Comment thread tests/test_message.py Outdated
{'forward_from_chat': Chat(-23, 'channel'),
'forward_from_message_id': 101,
'forward_date': datetime.now()},
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please reset the dict to the way it was.

@tsnoam

tsnoam commented Feb 10, 2018

Copy link
Copy Markdown
Member

The code itself LGTM.

However, I think it's over-engineering and only bloats the code. Though if the other maintainers find it suitable for ptb, I won't object.

@tsnoam

tsnoam commented Feb 12, 2018

Copy link
Copy Markdown
Member

@jh0ker thinks it a good idea to have this in the library. merging.

@tsnoam
tsnoam merged commit f0dfdfb into master Feb 12, 2018
@tsnoam
tsnoam deleted the reply-text-markdown-2 branch February 12, 2018 13:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

🔌 enhancement pr description: enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants