Add Javadoc documentation to all public headers#193
Merged
sgerbino merged 1 commit intocppalliance:developfrom Mar 5, 2026
Merged
Add Javadoc documentation to all public headers#193sgerbino merged 1 commit intocppalliance:developfrom
sgerbino merged 1 commit intocppalliance:developfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDeclaration-only PR expanding public and backend-facing APIs: adds exception helpers, scheduler run-control/query, timer_service lifecycle and callbacks, multiple backend implementation interfaces (timer, signal, resolver, socket, acceptor), TLS stream lifecycle and move semantics, plus small documentation edits. Changes
Sequence Diagram(s)Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
An automated preview of the documentation is available at https://193.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-03-05 20:56:47 UTC |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
include/boost/corosio/native/native_scheduler.hpp (1)
19-19: Brief should start with a verb.Per coding guidelines, brief documentation must begin with a verb (e.g., "Read data from the stream asynchronously."). The current brief starts with the noun "Scheduler".
Suggested fix
-/** Scheduler base that caches service pointers for native backends. +/** Caches service pointers for native backends.As per coding guidelines: "Brief documentation — One-sentence summary starting with a verb."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/native_scheduler.hpp` at line 19, Change the file-level/class brief to start with a verb: replace the current "Scheduler base that caches service pointers for native backends." with an imperative or present-tense verb phrase such as "Provide a scheduler base that caches service pointers for native backends." or "Cache service pointers for native backends in the scheduler base." Update the brief near the declaration for the scheduler (native_scheduler / Scheduler) in native_scheduler.hpp so it begins with a verb.include/boost/corosio/detail/except.hpp (1)
21-30: Consider adding explicit@param/precondition docs for pointer arguments.For
what(both overloads), document nullability/lifetime explicitly; forec, prefer@param ecover inline@p ecfor consistency with the header-wide Javadoc style.✍️ Suggested docs-only refinement
-/// Throw `std::logic_error` with the given message. +/// Throw `std::logic_error` with the given message. +/// `@param` what Pointer to a null-terminated message string. +/// Must not be null. -/// Throw `std::system_error` from `@p` ec. +/// Throw `std::system_error` from the specified error code. +/// `@param` ec Error code used to construct the exception. -/// Throw `std::system_error` from `@p` ec with the given context. +/// Throw `std::system_error` from the specified error code with context. +/// `@param` ec Error code used to construct the exception. +/// `@param` what Pointer to a null-terminated context string. +/// Must not be null.As per coding guidelines: “Docstrings should include a brief description … document all parameters, return values, and any preconditions, postconditions, or exceptions.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/detail/except.hpp` around lines 21 - 30, The function declarations throw_logic_error(char const* what), throw_system_error(std::error_code const& ec), and throw_system_error(std::error_code const& ec, char const* what) lack parameter documentation and preconditions; update their header comments to add explicit `@param` entries for ec and what (use "@param ec" not "@p ec"), state whether the pointer what may be null and any lifetime expectations (e.g., must remain valid until call returns), and document any preconditions or exceptions thrown by these functions so the header follows the project docstyle.include/boost/corosio/openssl_stream.hpp (1)
123-130: Consider expanded documentation for async operations (same as WolfSSL backend).Same recommendation as for
wolfssl_stream: iftls_streambase class contains comprehensive documentation for these virtual methods, these brief overrides suffice. Otherwise, the async operationshandshake()andshutdown()would benefit from documenting completion/error conditions and cancellation behavior per the coding guidelines.As per coding guidelines: "Brief documentation — One-sentence summary starting with a verb" is satisfied, but async operations warrant extended descriptions when the base class doesn't provide them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/openssl_stream.hpp` around lines 123 - 130, The three one-line overrides in openssl_stream.hpp (handshake(handshake_type), shutdown(), reset()) need expanded doc comments if the base class tls_stream does not already document async completion, error and cancellation semantics; update the comments for handshake() and shutdown() to state when the io_task completes, what errors may be propagated, and how cancellation is handled (matching the wolfssl_stream style), and keep reset()'s brief summary if tls_stream covers its semantics—refer to the tls_stream base docs and the wolfssl_stream comments to mirror wording and required details.include/boost/corosio/wolfssl_stream.hpp (1)
123-130: Consider expanded documentation for async operations.The brief comments for
handshake(),shutdown(), andreset()are good starting points. Since these areoverridemethods, verify the base classtls_streamcontains the comprehensive documentation (completion conditions, error conditions, cancellation behavior,@param/@return). If the base class documentation is complete, these briefs suffice; otherwise, consider adding:
- Completion conditions (when does the coroutine resume?)
- Error conditions (e.g.,
capy::cond::canceledfor cancellation)@paramforhandshake_type type- Preconditions for
reset()(must the session be shut down first?)As per coding guidelines: "Extended description — Short paragraph explaining what the function does, stating it is an asynchronous operation that suspends the calling coroutine until completion."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/wolfssl_stream.hpp` around lines 123 - 130, Check whether the base class tls_stream already documents completion, error and cancellation behavior; if it does not, expand the comments on handshake(handshake_type type), shutdown(), and reset() to include a short paragraph stating these are asynchronous coroutines that suspend the caller until completion, a `@param` describing handshake_type type, explicit completion conditions (when the coroutine resumes), error conditions (including capy::cond::canceled for cancellation and other failure modes), and any preconditions for reset() (e.g., whether the session must be shut down first); keep the comments concise but include those four elements so the overrides are fully specified relative to tls_stream.include/boost/corosio/resolver.hpp (1)
435-466: Minor: Consider moving theimplementationstruct to the existingpublic:section.The
implementationstruct is declared in a secondpublic:block (line 435), while there's already apublic:section starting at line 273. While this compiles correctly, consolidating to a singlepublic:section would improve readability.The documentation itself is well-written and follows the established backend interface pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/resolver.hpp` around lines 435 - 466, The implementation struct (struct implementation : io_object::implementation with methods resolve, reverse_resolve and cancel) is declared in a separate public: block; move this struct declaration into the existing earlier public section where the resolver's other public members are defined so there is a single consolidated public: section for readability and consistency with the class layout.include/boost/corosio/detail/timer_service.hpp (1)
89-90: Use a verb-led brief fortimer_service::callbackfor style consistency.Current summary starts with a noun phrase; switching to a verb-start sentence would align with the repo documentation convention.
📝 Suggested doc tweak
- /// Type-erased callback for earliest-expiry-changed notifications. + /// Notify earliest-expiry changes through a type-erased callback.Based on learnings: "Brief documentation — One-sentence summary starting with a verb."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/detail/timer_service.hpp` around lines 89 - 90, Update the brief doc comment for the class timer_service::callback to start with a verb (imperative) rather than a noun phrase; locate the class declaration named callback inside timer_service and replace the one-sentence summary with a verb-led phrase (e.g., "Notify about earliest-expiry changes." or similar) so it matches the project's documentation style guide.include/boost/corosio/tcp_acceptor.hpp (1)
426-432: Expandimplementation::acceptdocs with parameter and output-contract details.For this async backend entry point, the one-line brief does not describe pointer outputs, cancellation semantics, or resume-handle expectations.
📝 Suggested doc + signature clarity update
- /// Initiate an asynchronous accept operation. + /** Initiate an asynchronous accept operation. + + `@param` h The awaiting coroutine handle. + `@param` ex The executor used for resumption. + `@param` token The cancellation token for this operation. + `@param` ec_out Receives the completion error code. + `@param` peer_impl_out Receives the accepted peer implementation. + `@return` The coroutine handle to transfer control to. + */ virtual std::coroutine_handle<> accept( - std::coroutine_handle<>, - capy::executor_ref, - std::stop_token, - std::error_code*, - io_object::implementation**) = 0; + std::coroutine_handle<> h, + capy::executor_ref ex, + std::stop_token token, + std::error_code* ec_out, + io_object::implementation** peer_impl_out) = 0;As per coding guidelines: "@param documentation for each parameter ...
@returndocumentation — Describe the returned aggregate and its elements."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/tcp_acceptor.hpp` around lines 426 - 432, The accept virtual needs expanded documentation: add `@param` entries for the incoming std::coroutine_handle<> (describe which coroutine is being resumed/used and ownership expectations), capy::executor_ref (which executor the backend must use for I/O), std::stop_token (what cancellation triggers should do and whether accept must observe/throw on stop), std::error_code* (output contract: when to set error_code and lifetime expectations), and io_object::implementation** (output contract: when to allocate/assign the new implementation pointer and who owns/frees it). Also add an `@return` clause describing the returned std::coroutine_handle<> (what handle is returned to the caller, when it will be resumed, and any lifetime/ownership guarantees). Mention cancellation semantics (whether the function must detect stop and set error_code accordingly and whether it must resume or not) and explicitly state which symbols implement these behaviors (implementation::accept and the error_code*/io_object::implementation** outputs) so callers know how to implement and consume this backend entry point.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/boost/corosio/native/native.hpp`:
- Around line 12-14: Update the file-level Doxygen `@file` brief in
include/boost/corosio/native/native.hpp to start with a verb and be a
single-sentence summary; replace the current noun-phrase "Convenience header
that includes all native (devirtualized) public headers: I/O context, sockets,
acceptor, resolver, signal set, timer, and cancellation helpers." with an
active-voice sentence beginning with a verb (e.g., "Provide a convenience header
that includes all native (devirtualized) public headers: I/O context, sockets,
acceptor, resolver, signal set, timer, and cancellation helpers.") so the brief
follows the project's "start with a verb" doc convention and remains concise.
In `@include/boost/corosio/tcp_acceptor.hpp`:
- Around line 419-423: The doc comment for the TCP acceptor backend is
inaccurate: update the summary in include/boost/corosio/tcp_acceptor.hpp so it
matches the declared virtual API of the backend class (referencing the actual
backend interface name around the comment and the methods it declares, e.g.,
accept and option-management methods) and remove the incorrect mention of bind
and listen (or replace with the correct virtual methods if they exist); ensure
the brief description only lists the real abstract operations implemented by
platform backends.
In `@include/boost/corosio/tcp_server.hpp`:
- Around line 607-611: Update the doc comments for tcp_server(tcp_server&& o)
noexcept and tcp_server& operator=(tcp_server&& o) noexcept to describe the
parameter o (the source being moved from), state guarantees after the move (what
invariants the moved-from object preserves and any valid but unspecified state),
any preconditions (e.g., self-move prohibited or handled), and for operator=
include the `@return` contract (that it returns *this, reference to the assigned
object). Reference the symbols tcp_server(tcp_server&& o) and tcp_server&
operator=(tcp_server&& o) in the docs and keep the text concise: document
parameter o, postcondition on the moved-from object, exception/noexcept
behavior, and that operator= returns a reference to *this.
In `@include/boost/corosio/tls_stream.hpp`:
- Line 49: Update the brief comment that currently reads "TLS handshake role."
to a verb-first one to match project docs; modify the /// brief immediately
above the enum that documents the TLS handshake role to start with a verb (e.g.,
"Represent the TLS handshake role." or "Indicate the TLS handshake role.") so
the enum's one-sentence summary follows the verb-first Javadoc convention.
---
Nitpick comments:
In `@include/boost/corosio/detail/except.hpp`:
- Around line 21-30: The function declarations throw_logic_error(char const*
what), throw_system_error(std::error_code const& ec), and
throw_system_error(std::error_code const& ec, char const* what) lack parameter
documentation and preconditions; update their header comments to add explicit
`@param` entries for ec and what (use "@param ec" not "@p ec"), state whether the
pointer what may be null and any lifetime expectations (e.g., must remain valid
until call returns), and document any preconditions or exceptions thrown by
these functions so the header follows the project docstyle.
In `@include/boost/corosio/detail/timer_service.hpp`:
- Around line 89-90: Update the brief doc comment for the class
timer_service::callback to start with a verb (imperative) rather than a noun
phrase; locate the class declaration named callback inside timer_service and
replace the one-sentence summary with a verb-led phrase (e.g., "Notify about
earliest-expiry changes." or similar) so it matches the project's documentation
style guide.
In `@include/boost/corosio/native/native_scheduler.hpp`:
- Line 19: Change the file-level/class brief to start with a verb: replace the
current "Scheduler base that caches service pointers for native backends." with
an imperative or present-tense verb phrase such as "Provide a scheduler base
that caches service pointers for native backends." or "Cache service pointers
for native backends in the scheduler base." Update the brief near the
declaration for the scheduler (native_scheduler / Scheduler) in
native_scheduler.hpp so it begins with a verb.
In `@include/boost/corosio/openssl_stream.hpp`:
- Around line 123-130: The three one-line overrides in openssl_stream.hpp
(handshake(handshake_type), shutdown(), reset()) need expanded doc comments if
the base class tls_stream does not already document async completion, error and
cancellation semantics; update the comments for handshake() and shutdown() to
state when the io_task completes, what errors may be propagated, and how
cancellation is handled (matching the wolfssl_stream style), and keep reset()'s
brief summary if tls_stream covers its semantics—refer to the tls_stream base
docs and the wolfssl_stream comments to mirror wording and required details.
In `@include/boost/corosio/resolver.hpp`:
- Around line 435-466: The implementation struct (struct implementation :
io_object::implementation with methods resolve, reverse_resolve and cancel) is
declared in a separate public: block; move this struct declaration into the
existing earlier public section where the resolver's other public members are
defined so there is a single consolidated public: section for readability and
consistency with the class layout.
In `@include/boost/corosio/tcp_acceptor.hpp`:
- Around line 426-432: The accept virtual needs expanded documentation: add
`@param` entries for the incoming std::coroutine_handle<> (describe which
coroutine is being resumed/used and ownership expectations), capy::executor_ref
(which executor the backend must use for I/O), std::stop_token (what
cancellation triggers should do and whether accept must observe/throw on stop),
std::error_code* (output contract: when to set error_code and lifetime
expectations), and io_object::implementation** (output contract: when to
allocate/assign the new implementation pointer and who owns/frees it). Also add
an `@return` clause describing the returned std::coroutine_handle<> (what handle
is returned to the caller, when it will be resumed, and any lifetime/ownership
guarantees). Mention cancellation semantics (whether the function must detect
stop and set error_code accordingly and whether it must resume or not) and
explicitly state which symbols implement these behaviors (implementation::accept
and the error_code*/io_object::implementation** outputs) so callers know how to
implement and consume this backend entry point.
In `@include/boost/corosio/wolfssl_stream.hpp`:
- Around line 123-130: Check whether the base class tls_stream already documents
completion, error and cancellation behavior; if it does not, expand the comments
on handshake(handshake_type type), shutdown(), and reset() to include a short
paragraph stating these are asynchronous coroutines that suspend the caller
until completion, a `@param` describing handshake_type type, explicit completion
conditions (when the coroutine resumes), error conditions (including
capy::cond::canceled for cancellation and other failure modes), and any
preconditions for reset() (e.g., whether the session must be shut down first);
keep the comments concise but include those four elements so the overrides are
fully specified relative to tls_stream.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a7acf724-bb0d-483f-bc93-475e98530964
📒 Files selected for processing (18)
include/boost/corosio/detail/except.hppinclude/boost/corosio/detail/scheduler.hppinclude/boost/corosio/detail/timer_service.hppinclude/boost/corosio/io/io_object.hppinclude/boost/corosio/io/io_signal_set.hppinclude/boost/corosio/io/io_timer.hppinclude/boost/corosio/native/native.hppinclude/boost/corosio/native/native_scheduler.hppinclude/boost/corosio/openssl_stream.hppinclude/boost/corosio/resolver.hppinclude/boost/corosio/resolver_results.hppinclude/boost/corosio/signal_set.hppinclude/boost/corosio/tcp.hppinclude/boost/corosio/tcp_acceptor.hppinclude/boost/corosio/tcp_server.hppinclude/boost/corosio/tcp_socket.hppinclude/boost/corosio/tls_stream.hppinclude/boost/corosio/wolfssl_stream.hpp
|
GCOVR code coverage report https://193.corosio.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-03-05 21:04:27 UTC |
sgerbino
force-pushed
the
pr/doc-overview
branch
from
7fc13df to
dfcbfd1
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
include/boost/corosio/resolver.hpp (1)
436-465:⚠️ Potential issue | 🟡 MinorDocument the new resolver backend interface with full contract details.
Line 436 should be verb-first, and Lines 443-465 should include complete
@param/@returndocumentation for the new backend-facing virtuals.As per coding guidelines: “Brief documentation — One-sentence summary starting with a verb” and “Docstrings are required for all classes and functions in public headers ... document all parameters, return values...”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/resolver.hpp` around lines 436 - 465, Update the comment for the backend interface: make the one-sentence summary at the top of struct implementation verb-first (e.g., "Provide backend DNS resolution operations.") and add full Doxygen-style parameter and return documentation for each virtual method (implementation::resolve, implementation::reverse_resolve, implementation::cancel) documenting every parameter (the coroutine handle, capy::executor_ref, host/service or endpoint, flags, std::stop_token, std::error_code*, result pointer) and what the coroutine_handle<> return value represents, and mark cancel as noexcept with its effect; ensure each `@param` and `@return` is concise and follows the project's doc style.
🧹 Nitpick comments (2)
include/boost/corosio/detail/timer_service.hpp (2)
89-90: Use a verb-starting brief for the class doc line.Line 89 starts with a noun phrase; switch to a verb-start sentence (e.g., “Provide type-erased callback storage…”).
Based on learnings: “Brief documentation — One-sentence summary starting with a verb.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/detail/timer_service.hpp` around lines 89 - 90, Change the class comment for callback to start with a verb: replace the noun-phrase "Type-erased callback for earliest-expiry-changed notifications." with a verb-starting brief such as "Provide type-erased callback storage for earliest-expiry-changed notifications." so the one-line summary begins with a verb and clearly describes the purpose of the class callback.
174-208: Expand non-trivial API docs beyond one-line briefs.These lifecycle/cancellation methods are behavior-heavy; please add concise Javadoc details for thread-safety/concurrency context and parameter/return semantics where applicable.
As per coding guidelines: “Concurrency and overlap — State which operations may be simultaneously in flight” and “@param documentation for each parameter” plus “@return documentation.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/detail/timer_service.hpp` around lines 174 - 208, Expand the one-line docs for the lifecycle/cancellation APIs (shutdown, construct, destroy, destroy_impl, create_waiter, destroy_waiter, update_timer, insert_waiter, cancel_timer, cancel_waiter, cancel_one_waiter, process_expired) to include: what each function does, thread-safety guarantees (which may be called concurrently and which require external synchronization), which operations may run concurrently or overlap (e.g., update_timer vs. cancel_timer vs. process_expired), `@param` descriptions for parameters like implementation&, waiter_node*, time_point and any ownership/lifetime expectations, and `@return` semantics (what the returned size_t represents and when it can be zero). Reference the function names in the new comments and keep entries concise but specific about concurrency and return/parameter contracts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/boost/corosio/io/io_signal_set.hpp`:
- Around line 71-87: Change the struct brief to a verb-led one-sentence summary
(e.g., "Provide backend operations for signal-set wait and cancellation.") and
add full method-level doc comments for the virtuals on struct implementation:
for wait(coroutine_handle<>, capy::executor_ref, std::stop_token,
std::error_code*, int*) document each parameter (`@param`) describing the
coroutine handle, executor, stop token, optional error_code out-parameter and
optional signal-number out-parameter, the cancellation semantics when the
stop_token is triggered, and the return value meaning (std::coroutine_handle<>
resumed/resuming continuation or null); for cancel() add `@brief` and `@return`
(none) and describe that it cancels all pending wait operations and what
guarantees are provided to waiting coroutines (e.g., they will be resumed with
appropriate error_code or not) so the backend contract is complete and follows
the one-sentence verb-led brief rule.
In `@include/boost/corosio/openssl_stream.hpp`:
- Around line 123-130: Update the three minimal comments for
handshake(handshake_type), shutdown(), and reset() in openssl_stream.hpp to full
docstrings: describe each parameter (handshake_type), the asynchronous return
(capy::io_task<>), expected completion semantics (when the task completes
successfully), possible error cases and exceptions propagated on failure,
cancellation behavior, and any preconditions/postconditions (connection state
required before calling, whether the stream can be reused after completion),
plus thread-safety and reentrancy notes; ensure you reference the exact function
signatures handshake(handshake_type type) override, shutdown() override, and
reset() override so reviewers can find the docs easily.
In `@include/boost/corosio/signal_set.hpp`:
- Around line 163-178: Update the documentation for struct implementation (which
derives from io_signal_set::implementation) to use verb-first one-sentence
summary and add param/return contracts for its methods: for add(int
signal_number, flags_t flags) document `@param` signal_number the signal to
register, `@param` flags the registration flags and `@return` the std::error_code
indicating success or failure; for remove(int signal_number) document `@param`
signal_number the signal to unregister and `@return` the std::error_code result;
for clear() document that it unregisters all signals and `@return` the
std::error_code result; ensure the top-level doc for struct implementation is a
single verb-starting sentence (e.g., "Provide backend operations for signal set
management.") and apply terse docstyle consistent with public-header
requirements.
In `@include/boost/corosio/tcp_socket.hpp`:
- Around line 37-39: Update the header docstrings to follow the verb-first brief
style and add complete parameter/return contracts: change the noun-first brief
for native_handle_type to a verb-first one and likewise update the two other
noun-first briefs in this file; then expand the backend interface docblock(s)
that span the ~100–113 region to include `@param` and `@return` tags describing each
parameter, expected preconditions, ownership/borrowing semantics, error
conditions, and return value meanings so implementers know the contract for each
backend method.
---
Outside diff comments:
In `@include/boost/corosio/resolver.hpp`:
- Around line 436-465: Update the comment for the backend interface: make the
one-sentence summary at the top of struct implementation verb-first (e.g.,
"Provide backend DNS resolution operations.") and add full Doxygen-style
parameter and return documentation for each virtual method
(implementation::resolve, implementation::reverse_resolve,
implementation::cancel) documenting every parameter (the coroutine handle,
capy::executor_ref, host/service or endpoint, flags, std::stop_token,
std::error_code*, result pointer) and what the coroutine_handle<> return value
represents, and mark cancel as noexcept with its effect; ensure each `@param` and
`@return` is concise and follows the project's doc style.
---
Nitpick comments:
In `@include/boost/corosio/detail/timer_service.hpp`:
- Around line 89-90: Change the class comment for callback to start with a verb:
replace the noun-phrase "Type-erased callback for earliest-expiry-changed
notifications." with a verb-starting brief such as "Provide type-erased callback
storage for earliest-expiry-changed notifications." so the one-line summary
begins with a verb and clearly describes the purpose of the class callback.
- Around line 174-208: Expand the one-line docs for the lifecycle/cancellation
APIs (shutdown, construct, destroy, destroy_impl, create_waiter, destroy_waiter,
update_timer, insert_waiter, cancel_timer, cancel_waiter, cancel_one_waiter,
process_expired) to include: what each function does, thread-safety guarantees
(which may be called concurrently and which require external synchronization),
which operations may run concurrently or overlap (e.g., update_timer vs.
cancel_timer vs. process_expired), `@param` descriptions for parameters like
implementation&, waiter_node*, time_point and any ownership/lifetime
expectations, and `@return` semantics (what the returned size_t represents and
when it can be zero). Reference the function names in the new comments and keep
entries concise but specific about concurrency and return/parameter contracts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 65ed714a-98ff-455e-bb0e-9603d9bc584a
📒 Files selected for processing (18)
include/boost/corosio/detail/except.hppinclude/boost/corosio/detail/scheduler.hppinclude/boost/corosio/detail/timer_service.hppinclude/boost/corosio/io/io_object.hppinclude/boost/corosio/io/io_signal_set.hppinclude/boost/corosio/io/io_timer.hppinclude/boost/corosio/native/native.hppinclude/boost/corosio/native/native_scheduler.hppinclude/boost/corosio/openssl_stream.hppinclude/boost/corosio/resolver.hppinclude/boost/corosio/resolver_results.hppinclude/boost/corosio/signal_set.hppinclude/boost/corosio/tcp.hppinclude/boost/corosio/tcp_acceptor.hppinclude/boost/corosio/tcp_server.hppinclude/boost/corosio/tcp_socket.hppinclude/boost/corosio/tls_stream.hppinclude/boost/corosio/wolfssl_stream.hpp
✅ Files skipped from review due to trivial changes (1)
- include/boost/corosio/io/io_object.hpp
🚧 Files skipped from review as they are similar to previous changes (8)
- include/boost/corosio/detail/except.hpp
- include/boost/corosio/native/native_scheduler.hpp
- include/boost/corosio/wolfssl_stream.hpp
- include/boost/corosio/io/io_timer.hpp
- include/boost/corosio/tcp_acceptor.hpp
- include/boost/corosio/tcp_server.hpp
- include/boost/corosio/detail/scheduler.hpp
- include/boost/corosio/tcp.hpp
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #193 +/- ##
===========================================
+ Coverage 76.02% 76.26% +0.24%
===========================================
Files 98 99 +1
Lines 10534 10565 +31
Branches 2390 2422 +32
===========================================
+ Hits 8008 8057 +49
+ Misses 1801 1793 -8
+ Partials 725 715 -10
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 10 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
sgerbino
force-pushed
the
pr/doc-overview
branch
from
dfcbfd1 to
1892863
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
include/boost/corosio/io/io_timer.hpp (1)
86-113:⚠️ Potential issue | 🟡 MinorComplete the backend
waitcontract docs and make the brief imperative.The
implementationbrief andwaitdocs are currently too thin for a public async backend hook. Please switch the brief to a verb-led sentence and document@param/@returnplus cancellation semantics.📝 Suggested doc update
- /** Backend interface for timer wait operations. + /** Define backend hooks for timer wait operations. @@ - /// Initiate an asynchronous wait for the timer to expire. + /** Initiate an asynchronous wait for timer expiry. + + Suspend the calling coroutine until expiry or cancellation. + + `@param` h Coroutine handle to resume on completion. + `@param` ex Executor used to dispatch completion. + `@param` token Stop token for cooperative cancellation. + `@param` ec Output error code. + `@return` Coroutine handle to resume immediately. + */ virtual std::coroutine_handle<> wait(As per coding guidelines "Brief documentation — One-sentence summary starting with a verb" and "Docstrings are required for all classes and functions in public headers ... document all parameters, return values...".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/io/io_timer.hpp` around lines 86 - 113, Change the struct and method docs to imperative, complete API docs: rewrite the brief for struct implementation to a verb-led sentence (e.g., "Provide backend state and wait entry point for timer operations."), and expand the virtual wait(...) comment to document each parameter (the calling coroutine handle, capy::executor_ref, std::stop_token for cancellation, std::error_code* output), the return value (the coroutine_handle resumed when wait completes or cancelled), and explicit cancellation semantics (how stop_token cancelation is observed, what to set in error_code, and responsibility for clearing might_have_pending_waits_ and heap_index_). Ensure you reference the implementation::wait prototype and mention the fields expiry_, heap_index_, and might_have_pending_waits_ in the doc where relevant.include/boost/corosio/tcp_acceptor.hpp (1)
427-433:⚠️ Potential issue | 🟡 MinorDocument
acceptoutput and cancellation contract explicitly.Please expand this method doc to cover
@paramand@return, especially ownership/lifetime expectations for theio_object::implementation**out parameter and cancellation result expectations.As per coding guidelines "Docstrings are required for all classes and functions in public headers ... document all parameters, return values, and any preconditions, postconditions, or exceptions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/tcp_acceptor.hpp` around lines 427 - 433, The accept method lacks detailed documentation describing output and cancellation semantics; update the docstring for virtual std::coroutine_handle<> accept(...) to include `@param` descriptions for the coroutine handle, capy::executor_ref, std::stop_token (explain cancellation observer semantics and when cancellation can occur), std::error_code* (explain how and when errors are set), and io_object::implementation** (explicitly state ownership transfer, who allocates/frees the pointee, lifetime guarantees, and postconditions on success/failure), plus an `@return` description for the returned std::coroutine_handle<> describing what the handle represents and when it becomes ready; ensure you mention preconditions/postconditions (e.g., non-null out pointer requirement), cancellation result expectations (whether cancellation sets error_code or nulls the out param), and any thread-safety or ordering guarantees related to accept and the implementation pointer.include/boost/corosio/resolver.hpp (1)
436-465:⚠️ Potential issue | 🟡 MinorStrengthen backend async docs for
resolver::implementation.Please make the struct brief verb-led and add full contract docs for
resolveandreverse_resolve(@param,@return, string-view lifetime/copy behavior, and cancellation outcomes). This interface is public and currently under-documented.Based on learnings "Extended description — Short paragraph explaining what the function does, stating it is an asynchronous operation that suspends the calling coroutine until completion..." and "Cancellation — State that the operation supports cancellation via stop_token ... and that the resulting error compares equal to capy::cond::canceled."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/resolver.hpp` around lines 436 - 465, Update the documentation for struct implementation to be brief and verb-led and add full contract docs for resolve and reverse_resolve: state each is an asynchronous operation that suspends the calling coroutine until completion, document parameters (`@param`) for the coroutine_handle, capy::executor_ref, host/service (std::string_view) or endpoint, flags, std::stop_token, std::error_code* (out), and results pointer (resolver_results*/reverse_resolver_result*), clarify that string_view parameters are not stored beyond the call (caller must ensure lifetime or provide a copy), describe the return (std::coroutine_handle<> resumption token representing the pending operation), and explicitly specify cancellation behavior (operation supports cancellation via std::stop_token and when cancelled the error_code compares equal to capy::cond::canceled), and keep the cancel() doc terse stating it requests cancellation of any pending operations and is noexcept.
🧹 Nitpick comments (1)
include/boost/corosio/detail/timer_service.hpp (1)
89-90: Use a verb-led brief forcallbackto match doc convention.Consider rephrasing to an imperative summary (e.g., “Provide a type-erased callback for earliest-expiry change notifications.”).
As per coding guidelines "Brief documentation — One-sentence summary starting with a verb."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/detail/timer_service.hpp` around lines 89 - 90, The doc brief for the type-erased callback is not verb-led; update the one-line summary above class callback to start with a verb (imperative voice) — for example: "Provide a type-erased callback for earliest-expiry change notifications." — ensuring the short description follows the project's brief-documentation guideline and replaces the current noun-led sentence for class callback in timer_service.hpp.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/boost/corosio/detail/except.hpp`:
- Around line 21-30: Add missing Doxygen tags to the function declarations: for
throw_system_error(std::error_code const& ec) and
throw_system_error(std::error_code const& ec, char const* what) (and optionally
throw_logic_error(char const* what)) add `@param` entries describing ec and what,
and add `@throws` entries that state these functions throw std::system_error (for
the ec overloads) or std::logic_error (for throw_logic_error) and under what
preconditions (e.g., always throws / when called), so the declarations for
throw_system_error and throw_logic_error include explicit `@param` and `@throws`
documentation.
In `@include/boost/corosio/wolfssl_stream.hpp`:
- Around line 123-146: The public lifecycle methods handshake(handshake_type),
shutdown(), and reset() lack required async API contract details—update their
docstrings to include completion conditions (what success and failure mean and
returned error_code states), cancellation behavior (whether the awaitable
supports cancellation and its effects), notable error conditions (e.g., TLS
errors, I/O errors, and when error_code values are produced),
concurrency/serialization constraints (whether concurrent calls are allowed or
must be serialized), and preconditions/thread-safety notes (e.g., stream must be
open, reset may only be called when the underlying socket is closed or after
shutdown). Ensure each function's docs mention postconditions (state after
success/failure) and any exceptions or side-effects so callers know how to use
handshake, shutdown, and reset safely.
---
Outside diff comments:
In `@include/boost/corosio/io/io_timer.hpp`:
- Around line 86-113: Change the struct and method docs to imperative, complete
API docs: rewrite the brief for struct implementation to a verb-led sentence
(e.g., "Provide backend state and wait entry point for timer operations."), and
expand the virtual wait(...) comment to document each parameter (the calling
coroutine handle, capy::executor_ref, std::stop_token for cancellation,
std::error_code* output), the return value (the coroutine_handle resumed when
wait completes or cancelled), and explicit cancellation semantics (how
stop_token cancelation is observed, what to set in error_code, and
responsibility for clearing might_have_pending_waits_ and heap_index_). Ensure
you reference the implementation::wait prototype and mention the fields expiry_,
heap_index_, and might_have_pending_waits_ in the doc where relevant.
In `@include/boost/corosio/resolver.hpp`:
- Around line 436-465: Update the documentation for struct implementation to be
brief and verb-led and add full contract docs for resolve and reverse_resolve:
state each is an asynchronous operation that suspends the calling coroutine
until completion, document parameters (`@param`) for the coroutine_handle,
capy::executor_ref, host/service (std::string_view) or endpoint, flags,
std::stop_token, std::error_code* (out), and results pointer
(resolver_results*/reverse_resolver_result*), clarify that string_view
parameters are not stored beyond the call (caller must ensure lifetime or
provide a copy), describe the return (std::coroutine_handle<> resumption token
representing the pending operation), and explicitly specify cancellation
behavior (operation supports cancellation via std::stop_token and when cancelled
the error_code compares equal to capy::cond::canceled), and keep the cancel()
doc terse stating it requests cancellation of any pending operations and is
noexcept.
In `@include/boost/corosio/tcp_acceptor.hpp`:
- Around line 427-433: The accept method lacks detailed documentation describing
output and cancellation semantics; update the docstring for virtual
std::coroutine_handle<> accept(...) to include `@param` descriptions for the
coroutine handle, capy::executor_ref, std::stop_token (explain cancellation
observer semantics and when cancellation can occur), std::error_code* (explain
how and when errors are set), and io_object::implementation** (explicitly state
ownership transfer, who allocates/frees the pointee, lifetime guarantees, and
postconditions on success/failure), plus an `@return` description for the returned
std::coroutine_handle<> describing what the handle represents and when it
becomes ready; ensure you mention preconditions/postconditions (e.g., non-null
out pointer requirement), cancellation result expectations (whether cancellation
sets error_code or nulls the out param), and any thread-safety or ordering
guarantees related to accept and the implementation pointer.
---
Nitpick comments:
In `@include/boost/corosio/detail/timer_service.hpp`:
- Around line 89-90: The doc brief for the type-erased callback is not verb-led;
update the one-line summary above class callback to start with a verb
(imperative voice) — for example: "Provide a type-erased callback for
earliest-expiry change notifications." — ensuring the short description follows
the project's brief-documentation guideline and replaces the current noun-led
sentence for class callback in timer_service.hpp.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5a2a7bf3-ef96-405f-abf7-eb3c7161d52d
📒 Files selected for processing (18)
include/boost/corosio/detail/except.hppinclude/boost/corosio/detail/scheduler.hppinclude/boost/corosio/detail/timer_service.hppinclude/boost/corosio/io/io_object.hppinclude/boost/corosio/io/io_signal_set.hppinclude/boost/corosio/io/io_timer.hppinclude/boost/corosio/native/native.hppinclude/boost/corosio/native/native_scheduler.hppinclude/boost/corosio/openssl_stream.hppinclude/boost/corosio/resolver.hppinclude/boost/corosio/resolver_results.hppinclude/boost/corosio/signal_set.hppinclude/boost/corosio/tcp.hppinclude/boost/corosio/tcp_acceptor.hppinclude/boost/corosio/tcp_server.hppinclude/boost/corosio/tcp_socket.hppinclude/boost/corosio/tls_stream.hppinclude/boost/corosio/wolfssl_stream.hpp
✅ Files skipped from review due to trivial changes (1)
- include/boost/corosio/native/native.hpp
🚧 Files skipped from review as they are similar to previous changes (9)
- include/boost/corosio/native/native_scheduler.hpp
- include/boost/corosio/signal_set.hpp
- include/boost/corosio/io/io_object.hpp
- include/boost/corosio/tls_stream.hpp
- include/boost/corosio/detail/scheduler.hpp
- include/boost/corosio/openssl_stream.hpp
- include/boost/corosio/resolver_results.hpp
- include/boost/corosio/tcp_server.hpp
- include/boost/corosio/tcp_socket.hpp
sgerbino
force-pushed
the
pr/doc-overview
branch
from
1892863 to
2d77f29
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
include/boost/corosio/io/io_timer.hpp (1)
107-112:⚠️ Potential issue | 🟡 MinorDocument
implementation::waitwith full backend contract details.The new virtual wait entry point currently lacks parameter and return contract details, which makes backend behavior expectations ambiguous.
✏️ Proposed doc expansion
- /// Initiate an asynchronous wait for the timer to expire. + /** Initiate an asynchronous wait for timer expiry. + + `@param` h Coroutine handle to resume on completion. + `@param` ex Executor used to dispatch/resume completion. + `@param` token Stop token for per-wait cancellation. + `@param` ec Output error code set on completion. + + `@return` Coroutine handle to resume immediately, or a null/noop + handle when completion is deferred. + */ virtual std::coroutine_handle<> wait( std::coroutine_handle<>, capy::executor_ref, std::stop_token, std::error_code*) = 0;As per coding guidelines, “Docstrings are required for all classes and functions in public headers … document all parameters, return values, and any preconditions, postconditions, or exceptions.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/io/io_timer.hpp` around lines 107 - 112, Add a full docstring for implementation::wait describing each parameter and the return contract: explain that the first std::coroutine_handle<> is the awaiting coroutine to resume, capy::executor_ref is the executor on which resumption must occur, std::stop_token signals cancellation and must be checked by the backend, and std::error_code* is an optional out-parameter to report synchronous setup errors; state that the function returns a std::coroutine_handle<> representing the continuation to be resumed when the timer expires (or nullptr if completion is synchronous/cancelled), specify preconditions (timer is valid and executor is running), postconditions (either the returned handle will be resumed exactly once or the original awaiting coroutine is resumed via the provided executor; error_code is set on failure), thread-safety/cancellation semantics (how stop_token must be observed and race conditions handled), and any ownership/lifetime expectations for the timer and executor so implementers know how to implement backends consistently.include/boost/corosio/resolver.hpp (1)
436-465:⚠️ Potential issue | 🟡 MinorComplete backend interface docs for resolver operations.
For these new backend hooks, please add full
@param/@returncontract details and switch the struct brief to verb-first style.✏️ Proposed doc structure
- /** Backend interface for DNS resolution operations. + /** Define backend hooks for DNS resolution operations. @@ - /// Initiate an asynchronous forward DNS resolution. + /** Initiate an asynchronous forward DNS resolution. + `@param` h Coroutine handle to resume on completion. + `@param` ex Executor used for completion dispatch. + `@param` host Host query string. + `@param` service Service query string. + `@param` flags Resolution behavior flags. + `@param` token Stop token for cancellation. + `@param` ec Output error code. + `@param` out Output resolver results. + `@return` Coroutine handle to resume immediately, or null/noop + when completion is deferred. + */ @@ - /// Initiate an asynchronous reverse DNS resolution. + /** Initiate an asynchronous reverse DNS resolution. + `@param` h Coroutine handle to resume on completion. + `@param` ex Executor used for completion dispatch. + `@param` ep Endpoint to reverse-resolve. + `@param` flags Reverse-resolution behavior flags. + `@param` token Stop token for cancellation. + `@param` ec Output error code. + `@param` out Output reverse-resolution result. + `@return` Coroutine handle to resume immediately, or null/noop + when completion is deferred. + */ @@ - /// Cancel pending resolve operations. + /** Cancel pending resolve operations. */ virtual void cancel() noexcept = 0;As per coding guidelines, “Docstrings are required for all classes and functions in public headers … document all parameters, return values…” and “Brief documentation — One-sentence summary starting with a verb…”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/resolver.hpp` around lines 436 - 465, Change the struct brief to a verb-first sentence and add full `@param/`@return docs for implementation::resolve, implementation::reverse_resolve and implementation::cancel: document that resolve(...) initiates an async forward DNS lookup, explain each parameter (the caller coroutine_handle to resume on completion, capy::executor_ref used to run the operation, std::string_view host and service, resolve_flags flags, std::stop_token to request cancellation, std::error_code* out error code which must be non-null and is set on completion, resolver_results* out results which must be provided and is populated on success), and that it returns a std::coroutine_handle<> representing the continuation to resume on completion; do the same for reverse_resolve(...) describing endpoint const& ep, reverse_flags, reverse_resolver_result* out, and return semantics; for cancel() document it cancels pending operations, is noexcept, and does not throw; ensure docs clarify ownership/lifetime expectations (caller allocates the out pointers and they must remain valid until the continuation runs) and what error codes and result states callers should expect on success vs. cancellation/failure.
🧹 Nitpick comments (1)
include/boost/corosio/detail/timer_service.hpp (1)
201-201: Minor: Inconsistent whitespace in parenthetical note.The spaces inside the parentheses are inconsistent with typical style. Consider removing them.
📝 Suggested fix
- /// Cancel a single waiter ( stop_token callback path ). + /// Cancel a single waiter (stop_token callback path).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/detail/timer_service.hpp` at line 201, Update the comment for the cancellation path to remove extra spaces inside the parentheses so it matches project style: change the parenthetical from "( stop_token callback path )" to "(stop_token callback path)" in the comment above the Cancel a single waiter line in timer_service.hpp; ensure the rest of the comment text and punctuation remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/boost/corosio/native/native_scheduler.hpp`:
- Around line 19-30: Change the doc comments to use verb-first one-sentence
briefs: update the struct native_scheduler comment to begin with a verb (e.g.,
"Cache service pointers for native backends and reduce virtual calls.") and
update the member timer_svc_ comment to start with a verb (e.g., "Cache pointer
to the timer service, set during construction."). Ensure both summaries are
single-sentence, verb-led briefs that follow the project's brief-documentation
guideline.
---
Outside diff comments:
In `@include/boost/corosio/io/io_timer.hpp`:
- Around line 107-112: Add a full docstring for implementation::wait describing
each parameter and the return contract: explain that the first
std::coroutine_handle<> is the awaiting coroutine to resume, capy::executor_ref
is the executor on which resumption must occur, std::stop_token signals
cancellation and must be checked by the backend, and std::error_code* is an
optional out-parameter to report synchronous setup errors; state that the
function returns a std::coroutine_handle<> representing the continuation to be
resumed when the timer expires (or nullptr if completion is
synchronous/cancelled), specify preconditions (timer is valid and executor is
running), postconditions (either the returned handle will be resumed exactly
once or the original awaiting coroutine is resumed via the provided executor;
error_code is set on failure), thread-safety/cancellation semantics (how
stop_token must be observed and race conditions handled), and any
ownership/lifetime expectations for the timer and executor so implementers know
how to implement backends consistently.
In `@include/boost/corosio/resolver.hpp`:
- Around line 436-465: Change the struct brief to a verb-first sentence and add
full `@param/`@return docs for implementation::resolve,
implementation::reverse_resolve and implementation::cancel: document that
resolve(...) initiates an async forward DNS lookup, explain each parameter (the
caller coroutine_handle to resume on completion, capy::executor_ref used to run
the operation, std::string_view host and service, resolve_flags flags,
std::stop_token to request cancellation, std::error_code* out error code which
must be non-null and is set on completion, resolver_results* out results which
must be provided and is populated on success), and that it returns a
std::coroutine_handle<> representing the continuation to resume on completion;
do the same for reverse_resolve(...) describing endpoint const& ep,
reverse_flags, reverse_resolver_result* out, and return semantics; for cancel()
document it cancels pending operations, is noexcept, and does not throw; ensure
docs clarify ownership/lifetime expectations (caller allocates the out pointers
and they must remain valid until the continuation runs) and what error codes and
result states callers should expect on success vs. cancellation/failure.
---
Nitpick comments:
In `@include/boost/corosio/detail/timer_service.hpp`:
- Line 201: Update the comment for the cancellation path to remove extra spaces
inside the parentheses so it matches project style: change the parenthetical
from "( stop_token callback path )" to "(stop_token callback path)" in the
comment above the Cancel a single waiter line in timer_service.hpp; ensure the
rest of the comment text and punctuation remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4e0ce63d-0df8-46b7-ba8d-1914d9812fa9
📒 Files selected for processing (18)
include/boost/corosio/detail/except.hppinclude/boost/corosio/detail/scheduler.hppinclude/boost/corosio/detail/timer_service.hppinclude/boost/corosio/io/io_object.hppinclude/boost/corosio/io/io_signal_set.hppinclude/boost/corosio/io/io_timer.hppinclude/boost/corosio/native/native.hppinclude/boost/corosio/native/native_scheduler.hppinclude/boost/corosio/openssl_stream.hppinclude/boost/corosio/resolver.hppinclude/boost/corosio/resolver_results.hppinclude/boost/corosio/signal_set.hppinclude/boost/corosio/tcp.hppinclude/boost/corosio/tcp_acceptor.hppinclude/boost/corosio/tcp_server.hppinclude/boost/corosio/tcp_socket.hppinclude/boost/corosio/tls_stream.hppinclude/boost/corosio/wolfssl_stream.hpp
✅ Files skipped from review due to trivial changes (1)
- include/boost/corosio/signal_set.hpp
🚧 Files skipped from review as they are similar to previous changes (8)
- include/boost/corosio/detail/scheduler.hpp
- include/boost/corosio/tcp_server.hpp
- include/boost/corosio/detail/except.hpp
- include/boost/corosio/openssl_stream.hpp
- include/boost/corosio/tcp_acceptor.hpp
- include/boost/corosio/wolfssl_stream.hpp
- include/boost/corosio/io/io_object.hpp
- include/boost/corosio/tcp_socket.hpp
sgerbino
force-pushed
the
pr/doc-overview
branch
from
2d77f29 to
8e11d78
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
include/boost/corosio/io/io_object.hpp (1)
226-227: Data member brief uses noun phrase instead of verb form.The comment "The platform I/O handle owned by this object" is a noun phrase. While this reads naturally for a data member, the coding guidelines specify briefs should start with a verb.
Consider: "Store the platform I/O handle for this object." However, for a simple protected data member, the current phrasing is clear and this is a minor stylistic point.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/io/io_object.hpp` around lines 226 - 227, Update the brief comment for the data member h_ to use a verb form per coding guidelines: replace "The platform I/O handle owned by this object" with an imperative-style brief such as "Store the platform I/O handle for this object." Modify the comment adjacent to the declaration of handle h_ in io_object.hpp (symbol: h_) so it starts with a verb and retains the same meaning.include/boost/corosio/io/io_timer.hpp (2)
94-112: Data member briefs are acceptable;wait()documentation is good.The data member briefs (lines 94, 98, 101, 104) use noun/adjective phrases which read naturally for describing state. While strictly the guidelines call for verb-led briefs, for simple data members this is a minor stylistic point.
The
wait()method brief (line 107) correctly uses verb-led form ("Initiate an asynchronous wait..."). However, it lacks@paramdocumentation which the coding guidelines require for public headers.📝 Consider adding `@param` docs for wait()
/// Initiate an asynchronous wait for the timer to expire. + /** + `@param` h Coroutine handle to resume on completion. + `@param` ex Executor for dispatching the completion. + `@param` token Stop token for cancellation. + `@param` ec Output error code. + + `@return` Coroutine handle to resume immediately. + */ virtual std::coroutine_handle<> wait( std::coroutine_handle<>, capy::executor_ref, std::stop_token, std::error_code*) = 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/io/io_timer.hpp` around lines 94 - 112, The wait() declaration lacks `@param` documentation: add verb-led summary already present and then document each parameter for wait(std::coroutine_handle<>, capy::executor_ref, std::stop_token, std::error_code*) using `@param` tags that briefly describe the purpose of the coroutine handle (caller continuation), the capy::executor_ref (executor on which the wait runs), the std::stop_token (cancellation source), and the std::error_code* (optional out-parameter for error reporting), and ensure the tags appear in the io_timer.hpp public header alongside the existing brief for wait().
86-91: Struct brief uses noun phrase; consider verb-led form.The brief "Backend interface for timer wait operations" is a noun phrase. Per coding guidelines, documentation should start with a verb (e.g., "Provide the backend interface for timer wait operations" or "Define backend hooks for timer wait operations").
As per coding guidelines: "Brief documentation — One-sentence summary starting with a verb."
📝 Suggested fix
- /** Backend interface for timer wait operations. + /** Define backend hooks for timer wait operations. Holds per-timer state (expiry, heap position) and provides the virtual `wait` entry point that concrete timer services override. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/io/io_timer.hpp` around lines 86 - 91, Change the brief comment that currently reads "Backend interface for timer wait operations" to a verb-led sentence per the doc guideline; update the one-sentence summary above the struct (the io_timer struct declared immediately after this comment) to start with a verb such as "Provide the backend interface for timer wait operations" or "Define backend hooks for timer wait operations" so the brief is an imperative/verb-led sentence.include/boost/corosio/detail/timer_service.hpp (1)
89-114: Documentation forcallbackclass uses noun-phrase brief instead of verb-led.The brief on line 89 ("Type-erased callback...") uses a noun phrase. Per coding guidelines, briefs should start with a verb (e.g., "Provide a type-erased callback..."). The individual member briefs (lines 96, 99, 102, 108) correctly use verb-led style.
This is a minor inconsistency in a detail header; consider aligning for consistency.
📝 Suggested fix
- /// Type-erased callback for earliest-expiry-changed notifications. + /// Provide a type-erased callback for earliest-expiry-changed notifications.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/detail/timer_service.hpp` around lines 89 - 114, Update the class documentation brief to use a verb-led phrase: change the comment for the class callback to something like "Provide a type-erased callback for earliest-expiry-changed notifications." This targets the class callback declaration and leaves all member comments (callback(), callback(void*, void(*)(void*)), explicit operator bool() const, and void operator()() const) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/boost/corosio/detail/scheduler.hpp`:
- Around line 23-30: The class brief for scheduler uses a noun-first sentence;
change it to a verb-first one: edit the comment block above the scheduler
interface (the doc comment starting "/** Abstract interface for the event loop
scheduler.") and rewrite the first sentence to start with a verb (e.g., "Provide
an abstract interface for the event loop scheduler.") while keeping the rest of
the paragraph and `@see` references intact so the documentation remains the same
but follows the brief-doc guideline; update only that first sentence in the
scheduler class comment.
In `@include/boost/corosio/openssl_stream.hpp`:
- Around line 117-121: The move constructor and move assignment declarations for
openssl_stream are underspecified; update the docs for
openssl_stream(openssl_stream&&) noexcept and openssl_stream&
operator=(openssl_stream&&) noexcept to name the parameter (e.g. other),
describe the source-object post-move state (valid but unspecified or explicitly
empty/closed), state whether resources are transferred, document that move
assignment returns *this, and list any preconditions (self-move safety) and
noexcept guarantee; attach these notes as Doxygen-style comments immediately
above the two declarations so callers know the contract.
---
Nitpick comments:
In `@include/boost/corosio/detail/timer_service.hpp`:
- Around line 89-114: Update the class documentation brief to use a verb-led
phrase: change the comment for the class callback to something like "Provide a
type-erased callback for earliest-expiry-changed notifications." This targets
the class callback declaration and leaves all member comments (callback(),
callback(void*, void(*)(void*)), explicit operator bool() const, and void
operator()() const) unchanged.
In `@include/boost/corosio/io/io_object.hpp`:
- Around line 226-227: Update the brief comment for the data member h_ to use a
verb form per coding guidelines: replace "The platform I/O handle owned by this
object" with an imperative-style brief such as "Store the platform I/O handle
for this object." Modify the comment adjacent to the declaration of handle h_ in
io_object.hpp (symbol: h_) so it starts with a verb and retains the same
meaning.
In `@include/boost/corosio/io/io_timer.hpp`:
- Around line 94-112: The wait() declaration lacks `@param` documentation: add
verb-led summary already present and then document each parameter for
wait(std::coroutine_handle<>, capy::executor_ref, std::stop_token,
std::error_code*) using `@param` tags that briefly describe the purpose of the
coroutine handle (caller continuation), the capy::executor_ref (executor on
which the wait runs), the std::stop_token (cancellation source), and the
std::error_code* (optional out-parameter for error reporting), and ensure the
tags appear in the io_timer.hpp public header alongside the existing brief for
wait().
- Around line 86-91: Change the brief comment that currently reads "Backend
interface for timer wait operations" to a verb-led sentence per the doc
guideline; update the one-sentence summary above the struct (the io_timer struct
declared immediately after this comment) to start with a verb such as "Provide
the backend interface for timer wait operations" or "Define backend hooks for
timer wait operations" so the brief is an imperative/verb-led sentence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 15429c88-6fe2-41fd-b54a-1c7c9be6ec81
📒 Files selected for processing (18)
include/boost/corosio/detail/except.hppinclude/boost/corosio/detail/scheduler.hppinclude/boost/corosio/detail/timer_service.hppinclude/boost/corosio/io/io_object.hppinclude/boost/corosio/io/io_signal_set.hppinclude/boost/corosio/io/io_timer.hppinclude/boost/corosio/native/native.hppinclude/boost/corosio/native/native_scheduler.hppinclude/boost/corosio/openssl_stream.hppinclude/boost/corosio/resolver.hppinclude/boost/corosio/resolver_results.hppinclude/boost/corosio/signal_set.hppinclude/boost/corosio/tcp.hppinclude/boost/corosio/tcp_acceptor.hppinclude/boost/corosio/tcp_server.hppinclude/boost/corosio/tcp_socket.hppinclude/boost/corosio/tls_stream.hppinclude/boost/corosio/wolfssl_stream.hpp
🚧 Files skipped from review as they are similar to previous changes (5)
- include/boost/corosio/tls_stream.hpp
- include/boost/corosio/wolfssl_stream.hpp
- include/boost/corosio/detail/except.hpp
- include/boost/corosio/resolver.hpp
- include/boost/corosio/signal_set.hpp
Review and document every public class, struct, enum, function, type alias, and data member across 18 headers. Follows the project Javadoc conventions: /// for trivial briefs, /** */ for non-trivial, proper verb starts (Return, Construct, Check, etc.), and @see/@param where appropriate. Closes cppalliance#121
sgerbino
force-pushed
the
pr/doc-overview
branch
from
8e11d78 to
db50982
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review and document every public class, struct, enum, function, type alias, and data member across 18 headers. Follows the project Javadoc conventions: /// for trivial briefs, /** */ for non-trivial, proper verb starts (Return, Construct, Check, etc.), and @see/@param where appropriate.
Closes #121
Summary by CodeRabbit
New Features
Documentation