Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

API 4.7#1858

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Bibo-Joshi merged 17 commits intomasterfromapi-4.7
Apr 10, 2020
Merged

API 4.7#1858

Bibo-Joshi merged 17 commits intomasterfromapi-4.7
Apr 10, 2020

Conversation

Bibo-Joshi
Copy link
Member

@Bibo-JoshiBibo-Joshi commentedMar 30, 2020
edited
Loading

Closes#1856

Pure API updates:

  • Add classDice
  • AddBot.send_dice
  • AddMessage.dice
  • AddMessage.reply_dice
  • Add classBotCommand
  • AddBot.get_my_commands
  • AddBot.set_my_commands
  • AddBot.commands property
  • Add parametertgs_sticker toBot.create_new_sticker_set
  • Add parametertgs_sticker toBot.add_sticker_to_set
  • AddStickerSet.thumb
  • AddBot.set_sticker_set_thumb
  • Tests
  • Add warning about changed parameter order forBot.create_new_sticker_set andBot.add_sticker_to_set to release notes

New functionality:

  • AddFilters.dice to filter messages containing a dice
  • AllowFilters.dice(1) andFilters.dice([1, 5]) to filters for dices with specific values
  • Somehow allow to treat a dice message as text message with the corresponding emoji viaFilters.text andMessage.effective_message?
  • Tests

Problems I encountered:

  • I didn't have the means to nor did I feel like creating a custom animated sticker for testing purposes. So I just downloaded one vie TG desktop. Copyright concerns?
  • Bot.create_new_sticker_set was not tested before, I guess because we can't delete packs via the API. So I don't test with the new parameter, either.
  • Bot.upload_sticker_file seems to accept TGS files aspng_sticker. I'm jusing it ittest_bot_methods_1_tgs right now. Also pinged@botsupport about it. The resultingfile_id is not valid
  • While improving coverage I noticed, that themask_position foradd_sticker_to_set was untested. If I see correctly, request posts differently when posting files, which is the case for stickers, and thus themask_position has to beto_json-ed manually. Added that and the corresponding test.

@Bibo-JoshiBibo-Joshi added the ⚙️ bot-apiaffected functionality: bot-api labelMar 30, 2020
@Bibo-JoshiBibo-Joshi added this to the12.6 milestoneMar 30, 2020
Copy link
Member

@tsnoamtsnoam left a comment

Choose a reason for hiding this comment

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

Minor comments on the code, please excuse my short writing, I'm doing it from my phone.

Unitests are the most missed part here...

@Bibo-JoshiBibo-Joshi mentioned this pull requestMar 30, 2020
4 tasks
@Bibo-JoshiBibo-Joshi marked this pull request as ready for reviewApril 1, 2020 15:42
Copy link
Member

@tsnoamtsnoam left a comment

Choose a reason for hiding this comment

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

In addition to comments inline, please check the coverage report, I see some new code is not covered by tests.

Copy link
Member

@tsnoamtsnoam left a comment

Choose a reason for hiding this comment

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

we're getting close. a few minor details and we're set to go.

@Bibo-Joshi
Copy link
MemberAuthor

Bibo-Joshi commentedApr 8, 2020
edited
Loading

we're getting close. a few minor details and we're set to go.

About the not-tested lines:

  • one chunk isBot.create_new_sticker_set, which wasn't tested before (see PR desc)
  • the other is reply_markup inBot.stop_poll which also wasn't tested before (introduced inAll api 4.2 and 4.3 changes #1418 ). I test it now (at least I test, that passing a markup doesn't give errors.stop_poll returns only the Poll, not the complete message, so I can't check that the markup was actually updated.)

Copy link
Member

@tsnoamtsnoam left a comment

Choose a reason for hiding this comment

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

Ok. We're down to only docstrings 🥳
Lets discuss them over chat and we can merge...

Copy link
Member

@PoolitzerPoolitzer left a comment

Choose a reason for hiding this comment

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

minor suggestions


dice = _Dice()
"""Dice Messages. If an integer or a list of integers is passed, it filters messages to only
allow those whose dice value is appearing in the given list.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
allowthosewhosedicevalueisappearinginthegivenlist.
allowthosewithdicewherethevalueisappearinginthegivenlist.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

While this is slightly more accurate, the sentence feels kinda weird to me. And it should be clear from the context, that only dice messages are allowed. I'll not change for now. If this is important to you, you can PR for it ;)

@Bibo-Joshi
Copy link
MemberAuthor

CI fails unrelated. merging.

@Bibo-JoshiBibo-Joshi merged commitd63e710 intomasterApr 10, 2020
@Bibo-JoshiBibo-Joshi deleted the api-4.7 branchApril 10, 2020 17:22
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsAug 18, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@PoolitzerPoolitzerPoolitzer requested changes

@tsnoamtsnoamtsnoam approved these changes

Assignees
No one assigned
Labels
⚙️ bot-apiaffected functionality: bot-api
Projects
None yet
Milestone
12.6
Development

Successfully merging this pull request may close these issues.

API 4.7
3 participants
@Bibo-Joshi@tsnoam@Poolitzer

[8]ページ先頭

©2009-2025 Movatter.jp