Skip to content

Conversation

@radon-at-beeper
Copy link
Contributor

No description provided.

@coderabbitai
Copy link

coderabbitai bot commented Sep 22, 2025

📝 Walkthrough

Summary by CodeRabbit

  • Documentation
    • Clarified that deleting a bridge via Beeper client settings does not remove the local bridge database.
    • Added guidance to use bbctl delete to remove local data and noted the default local path (~/.local/share/bbctl/prod).
    • Explained implications of using custom locations (e.g., -l) where bbctl delete won’t remove the database.
    • Noted potential errors when re-adding a self-hosted bridge if the local database wasn’t deleted after bridge removal from Matrix rooms.

Walkthrough

Documentation update in README.md adds clarifications about bridge deletion behavior: distinguishes Beeper client bridge removal from local database removal, instructs on using bbctl delete, notes default/local database paths and -l option implications, and warns of possible errors when re-adding self-hosted bridges due to prior Matrix room removals.

Changes

Cohort / File(s) Summary of Changes
Docs: Bridge deletion clarifications
README.md
Added notes clarifying that removing a bridge in Beeper client settings does not delete the local bridge database; specify using bbctl delete for local cleanup, default path ~/.local/share/bbctl/prod, behavior when using -l to store elsewhere, and potential errors when re-adding self-hosted bridges after deletion due to Matrix room membership removals.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description Check ❓ Inconclusive No pull request description was provided in the PR body, so although the file-level summary shows README updates related to bridge deletion, the absence of an author-provided description leaves insufficient context about intent and scope. Request the author add a brief PR description summarizing the README changes, the reason for them, and any user-facing impact (for example local database paths and bbctl behavior) so reviewers have clear context.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "Document how to delete a bridge properly" succinctly and accurately reflects the primary change in the changeset, which updates README.md to explain how to remove a bridge and associated local database behavior; it is clear, focused, and suitable for someone scanning history.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch rr-document-db

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.


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

@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 (3)
README.md (3)

169-177: Clarify and tighten deletion behavior; use an admonition for the destructive caveat.

Rephrase and format as a warning with concise bullets. This improves scannability and reduces ambiguity around non-default data dirs.

-Note that deleting a bridge through the Beeper client settings will
-*not* delete the bridge database that is stored locally; you must
-delete that yourself, or use `bbctl delete` instead. The bridge
-databases are stored in `~/.local/share/bbctl/prod` by default.
-However, note that if you use any option that causes the bridge
-database to be stored in a separate location, such as `-l` which
-stores it in the current working directory, then `bbctl delete` will
-*not* delete the bridge database, and you will again have to delete it
-manually.
+> [!WARNING]
+> Deleting a bridge in Beeper client settings does not remove any local bridge database.
+>
+> - Default location: `~/.local/share/bbctl/prod`
+> - If you used `-l` (stores data in the current working directory) or configured a custom location, `bbctl delete` will not remove that database; delete it manually.
+>
+> To remove both server-side resources and local data (when using default locations for official bridges), prefer: `bbctl delete <name>`.

169-176: Verify flag/name accuracy and mention config reference.

Please confirm the exact flag(s) that alter the data directory (only -l, or also others like a config key). If applicable, add a pointer to ~/.config/bbctl.json so users can confirm where their data lives.

Apply this minimal tweak after confirming:

-If you used `-l` (stores data in the current working directory) or configured a custom location, `bbctl delete` will not remove that database; delete it manually.
+If you used `-l` (stores data in the current working directory) or configured a custom location (see `~/.config/bbctl.json`), `bbctl delete` will not remove that database; delete it manually.

179-182: Add remediation guidance for re-adding bridges to avoid stale room membership errors.

Offer an explicit next step so users know how to proceed safely.

-If you later re-add a self-hosted bridge after deleting it but not
-deleting the local database, you should expect errors, as the bridge
-will have been removed from Matrix rooms that it thinks it is a member
-of.
+If you later re-add a self-hosted bridge after deleting it but not deleting the local database, you should expect errors, as the bridge will have been removed from Matrix rooms that it thinks it is a member of. To avoid this, delete the local database first (or use `bbctl delete <name>` when applicable) before re-adding the bridge.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c3cb2a4 and 36c06b2.

📒 Files selected for processing (1)
  • README.md (1 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: deploy-docker
  • GitHub Check: build-docker (arm64)

@radon-at-beeper radon-at-beeper merged commit 10005ed into main Sep 22, 2025
11 checks passed
@radon-at-beeper radon-at-beeper deleted the rr-document-db branch September 22, 2025 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants