Skip to content

Conversation

@achowdhry-ripple
Copy link
Collaborator

@achowdhry-ripple achowdhry-ripple commented Dec 3, 2025

High Level Overview of Change

  • VoD feedback - README sample code in the xrpl-py repo has a bug
    The async example code creates my_tx_payment but then tries to submit my_tx_payment_signed which was never defined. It's missing this line of code before submitting my_tx_payment_signed:
    my_tx_payment_signed = sign(my_tx_payment,test_wallet)

^since submit_and_wait auto signs, this was likely just a variable typo during past edits

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

Did you update CHANGELOG.md?

  • Yes
  • No, this change does not impact library users

Test Plan

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

Walkthrough

The README.md asynchronous example has been updated to pass an unsigned transaction (my_tx_payment) to the submit_and_wait function instead of the previously signed version (my_tx_payment_signed).

Changes

Cohort / File(s) Summary
Documentation example update
README.md
Changed async example to pass unsigned transaction argument to submit_and_wait

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

  • Verify the corrected argument aligns with the actual function signature for submit_and_wait
  • Confirm this change reflects the intended API behavior for unsigned vs. signed transactions

Poem

🐰 A little hop, a small fix made,
Where unsigned now plays the trade,
In examples clear, the transaction flows,
Documentation blooms where correction grows! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'VoD request - fix example typo' is concise and clearly describes the main change: fixing a typo in a code example.
Description check ✅ Passed The description includes the required sections with sufficient detail: it explains the bug, identifies the root cause, and properly categorizes the change as a bug fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch vod-request-readme

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
README.md (1)

273-301: Fix correctly addresses the undefined variable issue.

Line 298 now correctly passes the unsigned my_tx_payment transaction to submit_and_wait with the test_wallet parameter, eliminating the undefined my_tx_payment_signed reference that was the bug described in the PR.

Minor observation: Consider consistency with sync examples.

The async example now follows the same pattern as the simple sync example (lines 31-40), passing an unsigned transaction with the wallet to submit_and_wait. However, the detailed sync example (lines 164-188) shows the alternative pattern: explicitly signing first with sign(), then submitting the signed transaction without the wallet. Both patterns appear valid, but this inconsistency in documentation could be confusing. If you're revising examples, consider aligning the approach across both sync and async sections for clarity.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 786db5e and 9de1f7c.

📒 Files selected for processing (1)
  • README.md (1 hunks)

@achowdhry-ripple achowdhry-ripple enabled auto-merge (squash) December 4, 2025 17:19
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