Sponsored Content
Skip to content

fix: Address code review comments regarding boost::coroutine2 - #6977

Merged
bthomee merged 2 commits into
developfrom
pratik/fix-coroutine2-review-comments
Apr 30, 2026
Merged

fix: Address code review comments regarding boost::coroutine2#6977
bthomee merged 2 commits into
developfrom
pratik/fix-coroutine2-review-comments

Conversation

@pratikmankawde

@pratikmankawde pratikmankawde commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

High Level Overview of Change

Minor cleanup updates to boost::coroutine2 PR.

Context of Change

This PR does some minor cleanups. No logic changes.
#6372

Test Plan

Tested locally.

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
@pratikmankawde pratikmankawde added the DraftRunCI Normally CI does not run on draft PRs. This opts in. label Apr 20, 2026
Comment thread conan/profiles/sanitizers
boost/*:without_context=False
# Boost stacktrace fails to build with some sanitizers
boost/*:without_stacktrace=True
{% elif "thread" in sanitizers %}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TSAN is inactive at the moment, so this is just to keep the profile consistent. This is a dead branch at the moment.

@codecov

codecov Bot commented Apr 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.6%. Comparing base (852fbe9) to head (cd3aa51).
⚠️ Report is 34 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           develop   #6977      +/-   ##
==========================================
+ Coverage     81.6%   82.6%    +1.0%     
==========================================
  Files         1010    1012       +2     
  Lines        75992   98196   +22204     
  Branches      7605   10594    +2989     
==========================================
+ Hits         62002   81112   +19110     
- Misses       13990   16687    +2697     
- Partials         0     397     +397     
Files with missing lines Coverage Δ
include/xrpl/core/Coro.ipp 100.0% <100.0%> (ø)

... and 380 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the codebase and build configuration to align with current Boost.Coroutine2 APIs and sanitizer-related Boost.Context build requirements.

Changes:

  • Replace deprecated boost::coroutines2::asymmetric_coroutine usage with boost::coroutines2::coroutine in JobQueue::Coro.
  • Centralize coroutine stack size into a named constant for clarity and reuse.
  • Adjust build/config tooling: remove BOOST_COROUTINES2_NO_DEPRECATION_WARNING, remove an ASAN-specific Conan fPIC override, and add Boost.Context ucontext configuration for TSAN in the sanitizer profile.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
include/xrpl/core/Coro.ipp Switch coroutine parameter type to boost::coroutines2::coroutine<void>::push_type and introduce a named stack-size constant.
conanfile.py Removes ASAN-specific fPIC override (but leaves an inert SANITIZERS env check).
conan/profiles/sanitizers Add TSAN-specific Boost.Context ucontext build flags for correct sanitizer annotations.
cmake/XrplInterface.cmake Removes BOOST_COROUTINES2_NO_DEPRECATION_WARNING compile definition.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread conanfile.py Outdated
Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
@pratikmankawde
pratikmankawde marked this pull request as ready for review April 29, 2026 16:10
@pratikmankawde pratikmankawde changed the title Addressing code review comments about boost::coroutine2 fix: Addressing code review comments about boost::coroutine2 Apr 29, 2026

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues.

Review by Claude Opus 4.6 · Prompt: V15

@pratikmankawde pratikmankawde added Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. and removed DraftRunCI Normally CI does not run on draft PRs. This opts in. labels Apr 30, 2026
@bthomee bthomee changed the title fix: Addressing code review comments about boost::coroutine2 fix: Address code review comments regarding boost::coroutine2 Apr 30, 2026
@bthomee
bthomee enabled auto-merge April 30, 2026 10:35
@bthomee
bthomee added this pull request to the merge queue Apr 30, 2026
Merged via the queue into develop with commit 6407f0f Apr 30, 2026
3 checks passed
@bthomee
bthomee deleted the pratik/fix-coroutine2-review-comments branch April 30, 2026 11:05
marek-foss-neti pushed a commit to marek-foss-neti/rippled that referenced this pull request May 5, 2026
…F#6977)

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
@mvadari mvadari added this to the 3.2.0 milestone May 20, 2026
beartec-jpg pushed a commit to beartec-jpg/FalconLedger that referenced this pull request Jun 1, 2026
…F#6977)

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants