Skip to content

Remove duplicate function try_pool_prop#3990

Merged
mulkieran merged 1 commit intostratis-storage:masterfrom
mulkieran:remove-try-pool-prop-fn
Mar 18, 2026
Merged

Remove duplicate function try_pool_prop#3990
mulkieran merged 1 commit intostratis-storage:masterfrom
mulkieran:remove-try-pool-prop-fn

Conversation

@mulkieran
Copy link
Copy Markdown
Member

@mulkieran mulkieran commented Mar 18, 2026

No description provided.

Its implementation is identical to that of pool_prop; so it is duplicate
code.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran mulkieran self-assigned this Mar 18, 2026
@mulkieran mulkieran moved this to In Progress in 2026March Mar 18, 2026
@mulkieran mulkieran moved this from In Progress to In Review in 2026March Mar 18, 2026
@mulkieran mulkieran marked this pull request as ready for review March 18, 2026 16:30
@mulkieran mulkieran added this to the v3.9.0 milestone Mar 18, 2026
@mulkieran mulkieran requested a review from jbaublitz March 18, 2026 16:44
@mulkieran
Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 18, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@packit-as-a-service
Copy link
Copy Markdown

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo dnf install -y 'dnf*-command(copr)'
  • dnf copr enable packit/stratis-storage-stratisd-3990-copr_pull
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2728ad14-331d-479a-b17a-0a016c63cc27

📥 Commits

Reviewing files that changed from the base of the PR and between 192bd41 and 61f7558.

📒 Files selected for processing (11)
  • src/dbus/pool/pool_3_0/mod.rs
  • src/dbus/pool/pool_3_1/mod.rs
  • src/dbus/pool/pool_3_2/mod.rs
  • src/dbus/pool/pool_3_3/mod.rs
  • src/dbus/pool/pool_3_4/mod.rs
  • src/dbus/pool/pool_3_5/mod.rs
  • src/dbus/pool/pool_3_6/mod.rs
  • src/dbus/pool/pool_3_7/mod.rs
  • src/dbus/pool/pool_3_8/mod.rs
  • src/dbus/pool/pool_3_9/mod.rs
  • src/dbus/pool/shared.rs
💤 Files with no reviewable changes (1)
  • src/dbus/pool/shared.rs

Walkthrough

Refactoring that removes the try_pool_prop wrapper function from the dbus pool shared module and replaces all call sites across ten pool version modules with direct calls to pool_prop.

Changes

Cohort / File(s) Summary
Pool version modules
src/dbus/pool/pool_3_0/mod.rs, src/dbus/pool/pool_3_1/mod.rs, src/dbus/pool/pool_3_2/mod.rs, src/dbus/pool/pool_3_3/mod.rs, src/dbus/pool/pool_3_4/mod.rs, src/dbus/pool/pool_3_5/mod.rs, src/dbus/pool/pool_3_6/mod.rs, src/dbus/pool/pool_3_7/mod.rs, src/dbus/pool/pool_3_8/mod.rs, src/dbus/pool/pool_3_9/mod.rs
Removed try_pool_prop from shared imports and replaced its usage with pool_prop in the total_physical_used property getter. Return types and signatures remain unchanged.
Shared pool utilities
src/dbus/pool/shared.rs
Deleted the try_pool_prop public async function that wrapped pool_prop functionality. The underlying pool_prop and set_pool_prop functions remain unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • zbus migration #3945: Directly related to the same refactoring effort—removes try_pool_prop and replaces call sites with pool_prop across the same dbus pool modules.

Suggested labels

maintenance

🚥 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 accurately describes the main change: removing the duplicate try_pool_prop function across all pool version modules and the shared module.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

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.

Tip

CodeRabbit can generate a title for your PR based on the changes with custom instructions.

Set the reviews.auto_title_instructions setting to generate a title for your PR based on the changes in the PR with custom instructions.

@mulkieran mulkieran merged commit 78407f2 into stratis-storage:master Mar 18, 2026
46 of 47 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in 2026March Mar 18, 2026
@mulkieran mulkieran deleted the remove-try-pool-prop-fn branch March 18, 2026 17:42
@mulkieran mulkieran moved this from Done to Done(2) in 2026March Mar 23, 2026
@mulkieran mulkieran moved this from Done(2) to Done(3) in 2026March Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done(3)

Development

Successfully merging this pull request may close these issues.

2 participants