Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
feat: 🎸 implement room announce#2527
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
Open
hcfw007 wants to merge1 commit intowechaty:mainChoose a base branch
base:main
Could not load branches
Branch not found:{{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
Open
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
huan requested changesJun 27, 2023
| typeWechatyEventListenerRoomTopic=(room:RoomInterface,newTopic:string,oldTopic:string,changer:ContactInterface,date?:Date)=>void|Promise<void> | ||
| typeWechatyEventListenerScan=(qrcode:string,status:PUPPET.types.ScanStatus,data?:string)=>void|Promise<void> | ||
| typeWechatyEventListenerStartStop=()=>void|Promise<void> | ||
| typeWechatyEventListenerRoomAnnounce=(room:RoomInterface,newAnnounce:string,changer:ContactInterface,oldAnnounce?:string,date?:Date)=>void|Promise<void> |
Member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please keep the parameters order the same as theWechatyEventListenerRoomTopic
Member
huan commentedMay 3, 2024
Hey@ellipsis, give me a code review |
OK! Reviewing this PR... Responding tothis comment by@huan. For more information about Ellipsis, check thedocumentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
👍 Looks good to me!
- Reviewed the entire pull request up to5d2ce40
- Looked at
124lines of code in3files - Took 1 minute and 9 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
3additional comments because they didn't meet confidence threshold of50%.
1.src/schemas/room-events.ts:113:
- Assessed confidence :
0% - Comment:
The implementation of theRoomEventListenerAnnounceand its addition to theRoomEventListenersinterface is correctly done, matching the existing patterns for event listeners in the system. - Reasoning:
The PR introduces a new event typeroom-announcein theRoomEventListenersinterface and the corresponding event listener typeRoomEventListenerAnnounce. The event listener type is defined correctly with appropriate parameters including the room, the new announcement, the changer, the old announcement, and the date. This aligns with the typical structure of event listeners in the system, which usually include the main object (room), the primary change (newAnnounce), the initiator of the change (changer), and optionally the previous state (oldAnnounce) and the timestamp (date).
2.src/schemas/wechaty-events.ts:210:
- Assessed confidence :
0% - Comment:
The implementation of theWechatyEventListenerRoomAnnounceand its addition to theWechatyEventListenersinterface is correctly done, matching the existing patterns for event listeners in the system. - Reasoning:
The PR introduces a new event typeroom-announcein theWechatyEventListenersinterface and the corresponding event listener typeWechatyEventListenerRoomAnnounce. The event listener type is defined correctly with appropriate parameters including the room, the new announcement, the changer, the old announcement, and the date. This aligns with the typical structure of event listeners in the system, which usually include the main object (room), the primary change (newAnnounce), the initiator of the change (changer), and optionally the previous state (oldAnnounce) and the timestamp (date).
3.src/wechaty-mixins/puppet-mixin.ts:421:
- Assessed confidence :
0% - Comment:
The implementation of theroom-announceevent handling in thepuppetMixinclass is correctly done, following the established patterns for similar event handlers in the system. - Reasoning:
The PR introduces handling for theroom-announceevent in thepuppetMixinclass. The handling logic includes finding the room and the changer based on their IDs, syncing the room, and emitting both a Wechaty and a room-specific event with the new announcement, the changer, the old announcement, and the date. This follows the typical pattern for event handling in the system, which involves resource lookup, state synchronization, and event emission with relevant data.
Workflow ID:wflow_UOORNsxRCg07wnkl
You can customize Ellipsis with review rules, user-specific overrides,quiet mode, and more. Seedocs.
⌛6 days left in your free trial,upgrade for $20/seat/month orcontact us.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading.Please reload this page.
issue:#2524
由于相关 PR 尚未 merge ， CI 无法通过。
Summary:
Implements the
room-announceevent handling in Wechaty, updating event schemas and the puppet mixin to support room announcement notifications.Key points:
RoomEventListenerAnnouncein/src/schemas/room-events.ts.WechatyEventListenerRoomAnnouncein/src/schemas/wechaty-events.ts.room-announcein/src/wechaty-mixins/puppet-mixin.ts.Generated with ❤️ byellipsis.dev