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

fix: buffer preparation in serializer #66

Merged

Conversation

ashtum
Copy link
Collaborator

@ashtum ashtum commented Feb 23, 2024

No description provided.

@ashtum ashtum force-pushed the fix-buffer-prepare-bug-in-serializer branch from 4b4f15d to c8755a1 Compare February 23, 2024 12:27
@ashtum ashtum changed the title fix: buffer prepare in serializer fix: buffer preparation in serializer Feb 23, 2024
Copy link

codecov bot commented Feb 23, 2024

Codecov Report

Attention: Patch coverage is 95.83333% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 86.24%. Comparing base (e1b4aa1) to head (6dec1c6).
Report is 3 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #66      +/-   ##
===========================================
+ Coverage    85.45%   86.24%   +0.78%     
===========================================
  Files           77       77              
  Lines         4269     4267       -2     
===========================================
+ Hits          3648     3680      +32     
+ Misses         621      587      -34     
Files Coverage Δ
src/serializer.cpp 72.52% <95.83%> (-0.05%) ⬇️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1b4aa1...6dec1c6. Read the comment docs.

@cmazakas
Copy link
Collaborator

We're going to probably want to add a test case here that demonstrates the issue.

@ashtum
Copy link
Collaborator Author

ashtum commented Feb 23, 2024

I'm going to add tests for using serializer with a http_proto::source instance. this should reproduce the issue.

@ashtum ashtum force-pushed the fix-buffer-prepare-bug-in-serializer branch from c8755a1 to fc47880 Compare February 25, 2024 09:48
@ashtum ashtum force-pushed the fix-buffer-prepare-bug-in-serializer branch 2 times, most recently from c50238f to aec832d Compare February 25, 2024 12:25
test/unit/serializer.cpp Outdated Show resolved Hide resolved
@ashtum ashtum force-pushed the fix-buffer-prepare-bug-in-serializer branch from aec832d to 8bf020e Compare February 25, 2024 12:41
@ashtum ashtum force-pushed the fix-buffer-prepare-bug-in-serializer branch from 8bf020e to 350ee7f Compare February 25, 2024 12:43
1 similar comment
@ashtum ashtum force-pushed the fix-buffer-prepare-bug-in-serializer branch from 350ee7f to 78e65b1 Compare February 25, 2024 14:37
@ashtum ashtum force-pushed the fix-buffer-prepare-bug-in-serializer branch from 78e65b1 to af59fce Compare February 28, 2024 16:50
@cmazakas
Copy link
Collaborator

The test changes seemed good on paper but I'm not sure I understand the problem this code is solving because I can comment things and tests will still pass for me locally so I think I need some clarification on what's going on here and how we're fixing it.

@ashtum ashtum force-pushed the fix-buffer-prepare-bug-in-serializer branch from af59fce to e9c8eac Compare March 2, 2024 10:11
@ashtum ashtum force-pushed the fix-buffer-prepare-bug-in-serializer branch 2 times, most recently from 19ae51e to acbb759 Compare March 2, 2024 11:44
1 similar comment
@ashtum ashtum force-pushed the fix-buffer-prepare-bug-in-serializer branch from acbb759 to 517b08a Compare March 2, 2024 15:22
test/unit/serializer.cpp Outdated Show resolved Hide resolved
@ashtum ashtum force-pushed the fix-buffer-prepare-bug-in-serializer branch from 517b08a to 9ca2431 Compare March 4, 2024 18:56
@ashtum ashtum force-pushed the fix-buffer-prepare-bug-in-serializer branch from 9ca2431 to 6dec1c6 Compare March 4, 2024 18:59
1 similar comment
@cmazakas cmazakas merged commit 5378e7a into cppalliance:develop Mar 6, 2024
53 checks passed
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.

4 participants