Skip to content

chore: revert "feat(ffi): add dash_spv_ffi_config_clear_peers (#591)"#593

Draft
xdustinface wants to merge 1 commit intov0.42-devfrom
chore/revert-clear-config-peers
Draft

chore: revert "feat(ffi): add dash_spv_ffi_config_clear_peers (#591)"#593
xdustinface wants to merge 1 commit intov0.42-devfrom
chore/revert-clear-config-peers

Conversation

@xdustinface
Copy link
Copy Markdown
Collaborator

@xdustinface xdustinface commented Mar 29, 2026

This reverts commit 472131a.

Summary by CodeRabbit

  • Refactor
    • Configuration management has been simplified, reducing the total number of API functions.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 29, 2026

📝 Walkthrough

Walkthrough

Removed the exported FFI function dash_spv_ffi_config_clear_peers from implementation, documentation, and tests. Function counts updated from 41 to 40 total functions and 17 to 16 configuration functions.

Changes

Cohort / File(s) Summary
Function Removal
dash-spv-ffi/src/config.rs, dash-spv-ffi/FFI_API.md, dash-spv-ffi/tests/test_config.rs
Removed dash_spv_ffi_config_clear_peers FFI function, including implementation with null-check logic, API documentation entry with C signature and safety notes, and two associated test cases (test_config_clear_peers and test_config_clear_peers_null).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 A function departs with peers now untamed,
No more shall we clear what was once proclaimed,
Three files simplified, the cleanup is done,
Less code to maintain—our refactor's won! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: reverting a previously added FFI function. It accurately reflects the changeset which removes dash_spv_ffi_config_clear_peers from documentation, source code, and tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/revert-clear-config-peers

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
Copy Markdown
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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
dash-spv-ffi/FFI_API.md (1)

1-7: ⚠️ Potential issue | 🟡 Minor

Consider documenting the rationale for this breaking change.

While "chore:" is technically valid for reverts, removing a public FFI function (dash_spv_ffi_config_clear_peers) is a breaking API change. The PR description provides no rationale for why this function is being removed.

Consider adding context about:

  • Why the function is being reverted (bug, design issue, not needed?)
  • Whether any clients are currently using this function
  • Migration guidance if this impacts existing users

As per coding guidelines: PRs are validated for title semantics, and "chore:" should accurately describe the changes. While reverting is typically a chore, the breaking nature of this change might warrant additional visibility in the PR description.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@dash-spv-ffi/FFI_API.md` around lines 1 - 7, Add a short rationale about the
breaking change: in the autogenerated FFI_API.md update the header or a
"Breaking changes" section to note that the public function
dash_spv_ffi_config_clear_peers was removed, why it was reverted (bug, design
decision, deprecated/unused), whether customers/clients are known to use it, and
provide migration guidance or alternatives (how clients should achieve the same
behavior or the planned replacement). Also update the PR title/description to
reflect this is a breaking revert (not just "chore:") and include the same
rationale and any compatibility notes so reviewers and users can assess impact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@dash-spv-ffi/FFI_API.md`:
- Around line 1-7: Add a short rationale about the breaking change: in the
autogenerated FFI_API.md update the header or a "Breaking changes" section to
note that the public function dash_spv_ffi_config_clear_peers was removed, why
it was reverted (bug, design decision, deprecated/unused), whether
customers/clients are known to use it, and provide migration guidance or
alternatives (how clients should achieve the same behavior or the planned
replacement). Also update the PR title/description to reflect this is a breaking
revert (not just "chore:") and include the same rationale and any compatibility
notes so reviewers and users can assess impact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3a03e661-22cc-4ec2-b77e-40fd4e7dc048

📥 Commits

Reviewing files that changed from the base of the PR and between 472131a and 1b0417c.

📒 Files selected for processing (3)
  • dash-spv-ffi/FFI_API.md
  • dash-spv-ffi/src/config.rs
  • dash-spv-ffi/tests/test_config.rs
💤 Files with no reviewable changes (2)
  • dash-spv-ffi/src/config.rs
  • dash-spv-ffi/tests/test_config.rs

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.02%. Comparing base (472131a) to head (1b0417c).

Additional details and impacted files
@@              Coverage Diff              @@
##           v0.42-dev     #593      +/-   ##
=============================================
- Coverage      67.06%   67.02%   -0.04%     
=============================================
  Files            318      318              
  Lines          67026    67020       -6     
=============================================
- Hits           44950    44921      -29     
- Misses         22076    22099      +23     
Flag Coverage Δ
core 75.21% <ø> (ø)
ffi 34.07% <ø> (-0.20%) ⬇️
rpc 19.92% <ø> (ø)
spv 83.73% <ø> (-0.06%) ⬇️
wallet 66.97% <ø> (ø)
Files with missing lines Coverage Δ
dash-spv-ffi/src/config.rs 61.49% <ø> (-1.20%) ⬇️

... and 6 files with indirect coverage changes

@github-actions github-actions bot added the ready-for-review CodeRabbit has approved this PR label Mar 29, 2026
@xdustinface xdustinface marked this pull request as draft March 30, 2026 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review CodeRabbit has approved this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant