Skip to content

Conversation

@jgmontoya
Copy link
Contributor

@jgmontoya jgmontoya commented Sep 10, 2025

Summary by CodeRabbit

  • Refactor

    • Consolidated multiple subscription setup routines into a single initialization step, simplifying startup flow while preserving existing behavior.
    • Improves reliability and error handling during initialization without altering functionality.
  • Chores

    • Internal API streamlined for clearer initialization sequencing.

No user-facing changes.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 10, 2025

Walkthrough

Adds a new public async method setup_all_subscriptions on Whitenoise that sequentially invokes existing private subscription setup routines. The initialization flow now calls this consolidated method instead of two separate calls. Underlying logic and error propagation remain unchanged.

Changes

Cohort / File(s) Summary of Changes
Whitenoise subscription setup consolidation
src/whitenoise/mod.rs
Added pub async fn setup_all_subscriptions(&'static Whitenoise) -> Result<()> that calls existing private methods (setup_global_users_subscriptions, setup_accounts_sync_and_subscriptions) in sequence; updated init to call the new method; no internal logic changes to the called methods.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Init as Initialization
    participant W as Whitenoise
    participant G as setup_global_users_subscriptions
    participant A as setup_accounts_sync_and_subscriptions

    Init->>W: setup_all_subscriptions()
    rect rgba(200, 230, 255, 0.3)
    note right of W: Consolidated subscription setup (new)
    W->>G: call
    G-->>W: Result
    W->>A: call
    A-->>W: Result
    end
    W-->>Init: Result
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • Refactor Signup Login #300 — Adjusts account setup to invoke per-account subscription setup, overlapping with this PR’s consolidation of global and account subscription initialization.

Suggested reviewers

  • delcin-raj
  • erskingardner

Pre-merge checks (3 passed)

✅ 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 succinctly describes the main change—making setup_all_subscriptions publicly available in the whitenoise/mod module—and follows a clear Conventional Commits style without extraneous details.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Poem

I twitch my whiskers, tidy and neat,
One hop instead of two—how sweet! 🐇
Subscriptions lined in single file,
I thump approval, ear-to-smile.
Less zig, more zag, in code I gleam—
A streamlined burrow for the team.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fe2891b and 4f96274.

📒 Files selected for processing (1)
  • src/whitenoise/mod.rs (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: check (macos-14, native)
  • GitHub Check: check (ubuntu-latest, native)
🔇 Additional comments (2)
src/whitenoise/mod.rs (2)

216-216: LGTM: Clean consolidation of subscription setup calls.

The change from separate method calls to a single consolidated method improves maintainability and provides a cleaner public interface.


226-230: LGTM: Well-structured public method consolidation.

The new setup_all_subscriptions method provides a clean public interface that:

  • Maintains the same sequential execution order as before
  • Properly propagates errors using the ? operator
  • Follows Rust naming conventions and async patterns
  • Enables external consumers to trigger subscription setup when needed
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/public-setup-all-subs

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.

@jgmontoya jgmontoya merged commit e70462b into master Sep 10, 2025
4 checks passed
@jgmontoya jgmontoya deleted the feat/public-setup-all-subs branch September 10, 2025 18:16
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.

3 participants