Skip to content
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

Implement filtering by chat type #203

Merged
merged 14 commits into from
May 27, 2022
Merged

Implement filtering by chat type #203

merged 14 commits into from
May 27, 2022

Conversation

KnorpelSenf
Copy link
Member

@KnorpelSenf KnorpelSenf commented May 14, 2022

TODO:

  • narrow down context type
  • add JSDoc

Closes #105.

@KnorpelSenf
Copy link
Member Author

This is blocked by #100. The narrowed context for chat types should be exposed. Will wait in order to prevent merge conflicts.

@KnorpelSenf
Copy link
Member Author

Types should ideally also reflect that channel posts are sent in channels, and messages aren't. There may be a lot more dependencies between these things. This will get a very complex type annotation, but it is probably worth it.

@KnorpelSenf KnorpelSenf marked this pull request as ready for review May 19, 2022 19:21
@KnorpelSenf
Copy link
Member Author

Types should ideally also reflect that channel posts are sent in channels, and messages aren't. There may be a lot more dependencies between these things. This will get a very complex type annotation, but it is probably worth it.

In fact, this should rather be done in @grammyjs/types because grammY would automatically reflect these changes. Also, we never unset properties. It is good enough that they are optional, because user code cannot really break in that case anyway.

Can be merged as-is after an approval.

@KnorpelSenf
Copy link
Member Author

Thanks for the review and the suggestion about optional types! I'll merge this as soon as we have a second approval :)

@all-contributors add @kolay-v for review

@allcontributors
Copy link
Contributor

@KnorpelSenf

I've put up a pull request to add @kolay-v! 🎉

Copy link
Member

@EdJoPaTo EdJoPaTo left a comment

Choose a reason for hiding this comment

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

I like this as it both simplifies some code and provides better typings. Worked in the bots I tried it both without changes and with using the new chatType method.

src/composer.ts Outdated Show resolved Hide resolved
@KnorpelSenf KnorpelSenf requested a review from EdJoPaTo May 27, 2022 19:13
src/composer.ts Outdated Show resolved Hide resolved
Co-authored-by: EdJoPaTo <[email protected]>
@KnorpelSenf
Copy link
Member Author

Thanks for the reviews!

@KnorpelSenf KnorpelSenf merged commit b8ac3d6 into main May 27, 2022
@KnorpelSenf KnorpelSenf deleted the composer-chat-type branch May 27, 2022 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

feat: Filtering for chat type
3 participants