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

stream prepare has nullary overload #81

Merged
merged 3 commits into from
Apr 15, 2024

Conversation

cmazakas
Copy link
Collaborator

@cmazakas cmazakas commented Apr 8, 2024

No description provided.

@vinniefalco
Copy link
Member

Well shouldn't we remove the 1-arg overload?

Copy link

codecov bot commented Apr 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.37%. Comparing base (b5cbccc) to head (b2b46e8).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #81      +/-   ##
===========================================
+ Coverage    89.23%   89.37%   +0.13%     
===========================================
  Files           77       77              
  Lines         4339     4348       +9     
===========================================
+ Hits          3872     3886      +14     
+ Misses         467      462       -5     
Files Coverage Δ
include/boost/http_proto/serializer.hpp 97.43% <ø> (ø)
src/serializer.cpp 92.03% <100.00%> (+2.27%) ⬆️

... and 1 file 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 b5cbccc...b2b46e8. Read the comment docs.

last_chunk_len_ =
1 + // "0"
crlf_len_ +
crlf_len_; // chunked-body termination requires an extra CRLF

static
constexpr
std::size_t
chunked_overhead_ =
Copy link
Collaborator

Choose a reason for hiding this comment

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

We might be able to use this constant in serializer::prepare() and serializer::capacity() too.

@cmazakas
Copy link
Collaborator Author

cmazakas commented Apr 9, 2024

Well shouldn't we remove the 1-arg overload?

Good question. I didn't see a reason to outright remove it.

I suppose users can take the return from stream::preapre() and then use Buffers to manipulate it. If you want, I can go ahead and remove the stream::preapre(std::size_t); overload

// last-chunk
// this enables users to call:
//
// stream.commit(n); stream.close();
Copy link
Member

Choose a reason for hiding this comment

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

Nice comment... yes, this is a good goal

1 similar comment
@cmazakas cmazakas merged commit 7bd53ed into cppalliance:develop Apr 15, 2024
53 checks passed
@cmazakas cmazakas deleted the feature/nullary-prepare branch April 15, 2024 15:54
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.

None yet

4 participants