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.

Sign up for GitHub

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

Jump to bottom

feat: add messages api #1720

Merged
merged 1 commit into from
Dec 3, 2024
Merged

feat: add messages api #1720

merged 1 commit into from
Dec 3, 2024

Conversation

namchuai
Copy link
Collaborator

@namchuai namchuai commented Nov 25, 2024 •
edited
Loading

Describe Your Changes

Self Checklist

  • Added relevant comments, esp in complex areas
  • Updated docs (for bug fixes / features)
  • Created issues for follow-up changes or refactoring needed

@namchuai namchuai force-pushed the j/add-message branch 5 times, most recently from 7b6126f to 92b845b Compare November 27, 2024 16:58
@namchuai namchuai changed the title J/add message feat: add messages api Nov 27, 2024
@namchuai namchuai marked this pull request as ready for review November 27, 2024 17:00
@namchuai namchuai requested a review from a team November 27, 2024 17:00
@vansangpfiev
Copy link
Contributor

Great work!
It would be nice if we have unit tests for ToJson functions.
Also, I think can consider moving the implementation to the cc files to reduce the compilation time, especially on Windows.

@namchuai namchuai force-pushed the j/add-message branch 2 times, most recently from b18891d to f50a974 Compare November 29, 2024 08:53
@namchuai
Copy link
Collaborator Author

I will add the test in the following PR

@namchuai namchuai force-pushed the j/add-message branch 3 times, most recently from 2578833 to 4f4b85e Compare December 3, 2024 14:47
@namchuai namchuai merged commit 0528553 into dev Dec 3, 2024
5 checks passed
@namchuai namchuai deleted the j/add-message branch December 3, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vansangpfiev vansangpfiev vansangpfiev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@namchuai @vansangpfiev