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

Add chat member filters #10

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add chat member filters #10

wants to merge 7 commits into from

Conversation

rayz1065
Copy link
Member

This pull request adds chat member filters to the plugin, a convenient way to filter for chat member statuses and updates.
I moved the original code to a separate storage.ts file and created a filters.ts file containing the new filters, everything is exported from mod.ts.
I updated the README to explain how the new filters can be used.

roziscoding
roziscoding previously approved these changes Jan 28, 2025
Copy link
Collaborator

@roziscoding roziscoding left a comment

Choose a reason for hiding this comment

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

This is awesome work.
TYSM!

Copy link
Member

@KnorpelSenf KnorpelSenf left a comment

Choose a reason for hiding this comment

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

Implementation looks great!

Regarding the docs: It's awesome to have such great descriptions, but they are not duplicated in the repo and the website. In general, we tend to only give very short quickstarts in the README files and link to the website for everything else. This makes sure that the docs don't start to diverge and it also reduces the maintenance overhead.

You may or may not remove them, do whatever you think is best. That being said, you might want to save the effort in the future.

I just have a few nitpicks, the changes LGTM otherwise:

KnorpelSenf
KnorpelSenf previously approved these changes Jan 30, 2025
Copy link
Member

@KnorpelSenf KnorpelSenf left a comment

Choose a reason for hiding this comment

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

LGTM after you and @roziscoding have figured out how to prevent line wrapping

Copy link
Member

@KnorpelSenf KnorpelSenf left a comment

Choose a reason for hiding this comment

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

LGTM, @roziscoding?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants