Skip to content

Done Basic messaging functionality #26

Merged
Prakhar896 merged 8 commits intomainfrom
nicholas
Jun 27, 2024
Merged

Done Basic messaging functionality #26
Prakhar896 merged 8 commits intomainfrom
nicholas

Conversation

@nicholascheww
Copy link
Copy Markdown
Contributor

No description provided.

@nicholascheww nicholascheww requested a review from Prakhar896 June 27, 2024 10:03
@nicholascheww nicholascheww self-assigned this Jun 27, 2024
@Prakhar896
Copy link
Copy Markdown
Contributor

Bro why don't you have any PR notes? @nicholascheww

@Prakhar896 Prakhar896 added the enhancement New feature or request label Jun 27, 2024
Copy link
Copy Markdown
Contributor

@Prakhar896 Prakhar896 left a comment

Choose a reason for hiding this comment

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

Impressive and very well-written. Just one question, see my comment.

ws.send(JSON.stringify(message));
}

ws.on("message", (message) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this supposed to be nested in the wss.on("connection") thing? I'm unsure so just asking and double-checking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes cause when they reach the /chat endpoint they will connect to the ws server, which i want the messaged to be rendered then

@nicholascheww nicholascheww requested a review from Prakhar896 June 27, 2024 13:35
Copy link
Copy Markdown
Contributor

@Prakhar896 Prakhar896 left a comment

Choose a reason for hiding this comment

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

LGTM

@Prakhar896 Prakhar896 merged commit 936ec15 into main Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants