Skip to content

Add separate BasicFrame and BasicFrameWithLen implementations for C and C++ - #52

Merged
rijesha merged 6 commits into
mainfrom
copilot/review-c-cpp-implementations
Nov 29, 2025
Merged

Add separate BasicFrame and BasicFrameWithLen implementations for C and C++#52
rijesha merged 6 commits into
mainfrom
copilot/review-c-cpp-implementations

Conversation

Copilot AI commented Nov 29, 2025

Copy link
Copy Markdown
Contributor
  • Analyze issue requirements for C and C++ BasicFrame/BasicFrameWithLen implementations
  • Review current C boilerplate code (struct_frame_default_frame.h, struct_frame_types.h, etc.)
  • Review current C++ boilerplate code (struct_frame.hpp, struct_frame_types.hpp, etc.)
  • Understand the frame format specifications from frame_formats.proto
  • Run existing tests to confirm baseline functionality
  • Create separate, cleaner C boilerplate files for BasicFrame (with msg_id to length lookup)
  • Create separate, cleaner C boilerplate files for BasicFrameWithLen (length in packet)
  • Create matching C++ boilerplate files for BasicFrame and BasicFrameWithLen
  • Add encode and decode in buffer implementations for C and C++
  • Test new implementations locally
  • Fix code review feedback (consistent API for reserve/finish)
  • Update C tests to use new BasicFrame
  • Update C++ tests to use new BasicFrame
  • Update Python boilerplate with BasicFrame class
  • Update Python tests to use BasicFrame
  • Update TypeScript tests to use BasicFrame format
  • Update JavaScript tests to use BasicFrame format
  • Verify 100% cross-platform compatibility
  • Run code review and CodeQL checker
Original prompt

This section details on the original issue you should resolve

<issue_title>Review and improve the C++ and C implementations</issue_title>
<issue_description>Review the C++ and C implementations parsing and encoding implementations. They are a bit disjointed. We should ensure that that the BasicFrameWithLen and BasicFrame are implemented in both languages. The BasicFrame needs to be able to translate a list of msg_ids to msg_lengths.

The c code seems very complicated right now. It may be that we should have separate boiler plate code for the BasicFrameWithLen and BasicFrame implementations as that may simplify the implementation and make it more easy to use and understand. </issue_description>

Comments on the Issue (you are @copilot in this section)

@rijesha Ensure that each language supports the BasicFrameWithLen and BasicFrame. Only c and c++ needs to support the encode and decode in buffer implementations.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits November 29, 2025 22:21
Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com>
…nd C++

Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com>
…nused code

Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com>
Copilot AI changed the title [WIP] Review and improve C and C++ parsing and encoding implementations Add separate BasicFrame and BasicFrameWithLen implementations for C and C++ Nov 29, 2025
Copilot AI requested a review from rijesha November 29, 2025 22:34
@rijesha

rijesha commented Nov 29, 2025

Copy link
Copy Markdown
Contributor

@copilot have the cross compatibilty test use this new BasicFrame.

Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com>

Copilot AI commented Nov 29, 2025

Copy link
Copy Markdown
Contributor Author

@copilot have the cross compatibilty test use this new BasicFrame.

Updated all cross-platform tests (C, C++, Python, TypeScript, JavaScript) to use the new BasicFrame implementation with the 2-start-byte format. The compatibility matrix now shows 100% success rate (25/25). Commit d951163.

Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com>
@rijesha
rijesha marked this pull request as ready for review November 29, 2025 23:27
@rijesha
rijesha merged commit 2b4938c into main Nov 29, 2025
2 checks passed
@rijesha
rijesha deleted the copilot/review-c-cpp-implementations branch December 6, 2025 03:20
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.

Review and improve the C++ and C implementations

2 participants