doc: Restructure documentation and add User Guide by bmr-cymru · Pull Request #454 · snapshotmanager/snapm

bmr-cymru · 2025-08-29T20:07:28Z

Major Changes:

Split documentation: Moved detailed content from README to new comprehensive User Guide (RST format for Sphinx)
Added Quick Start section: README now focuses on getting users started in minutes
Standardized command examples: All examples now use # root prompt instead of sudo
Updated installation methods: Replaced python setup.py install with pip install .
Harmonized terminology: Use "snapshot set" in descriptive text, "snapset" only for UI commands

README.md Changes:

Streamlined to focus on discovery and onboarding
Added practical Quick Start guide with common workflows
Added proper Contributing, Support, and Requirements sections
Added storage backend comparison table
Removed verbose command reference (moved to User Guide)

New User Guide (docs/user-guide.rst):

Complete command reference with examples
Troubleshooting section with common issues
Best practices and security considerations
Integration examples with other tools
Performance tuning guidansoce
Field reference and exit codes

Language/Style Fixes:

Removed uncertain language ("normally operate" → "operate")
Fixed typos ("Snaphot" → "Snapshot")
Standardized all code examples to use consistent formatting
Maintained accurate field names (nr_snapshots, size_bytes, free_bytes)

Necessary updates:

Fixup doc/conf.py, doc/index.rst, man pages, and other assets that are affected by the main docs changes.

Resolves: #237
Resolves: #439
Resolves: #469
Resolves: #468

Summary by CodeRabbit

Documentation
- README redesigned for end users: Apache‑2.0 badge/link, tightened license text, concise intro, feature‑focused layout (Features, Quick Start, Installation (RPM/manual), Usage, Boot & recovery), Related Projects and Public resources added.
- New comprehensive user guide: CLI reference, snapshot set concepts, size policies, supported backends table, scheduling, bootable snapshots, plugin architecture, examples, JSON/tabular outputs, troubleshooting.
- Man page: unified “snapshot set” terminology, expanded report/JSON fields (adds basename/index), updated examples and validation notes.
- Doc config/title/date updated for Snapm v0.4.0.
Chores
- CONTRIBUTING expanded with developer setup, test and docs build guidance; installation and systemd steps clarified.

coderabbitai · 2025-08-29T20:07:35Z

Walkthrough

Reorganizes and expands documentation: rewrites README into an installation-first Quick Start, adds a comprehensive Sphinx user guide, updates Sphinx config and index, expands CONTRIBUTING, and revises the man page to prefer “snapshot set” terminology and richer JSON/field examples. Documentation-only changes; no runtime or public API edits.

Changes

Cohort / File(s)	Summary
README overhaul `README.md`	Rewrote README: added license badge and Apache-2.0 link; new Features, Quick Start, Installation (RPM & Manual), Basic Usage, Supported Storage Backends, Documentation/Support/Contributing; moved detailed CLI reference into the new Sphinx user guide.
Sphinx config & index `doc/index.rst`, `doc/conf.py`	`doc/index.rst`: added `:caption: Documentation`, replaced `snapm` entry with `user_guide`, removed plugin-architecture paragraph. `doc/conf.py`: removed direct `snapm` import, added `autodoc_mock_imports = ['boom','dbus','dbus_client_gen','dbus_python_client_gen']`, updated `html_title` to `Snapm v0.4.0`, bumped copyright to 2025.
New user guide `doc/user_guide.rst`	Added comprehensive Sphinx user guide: architecture, key concepts (snapshot sets, size policies), full CLI reference and examples, reporting options, workflows, troubleshooting, backend appendix, and exit codes.
Man page updates `man/man8/snapm.8`	Reworded to consistently use “snapshot set”; added `basename` and `index` fields; expanded JSON output examples to a structured `Snapsets` top-level; clarified status-precedence semantics; updated option/argument wording (e.g., `sort_fields`) and examples; documented underscore restriction in names.
Contributor docs `CONTRIBUTING.md`	Expanded developer setup: prerequisites (dnf builddep/install), install `boom-boot`, virtualenv guidance (`--system-site-packages`), editable install (`pip install -e .`), run-from-clone PATH/PYTHONPATH instructions, un-commented setup/test commands, test/cleanup guidance, and documentation build steps.

Sequence Diagram(s)

(Documentation-only changes; no runtime/control-flow modifications — sequence diagram not applicable.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

doc: update README.md overview #237 — README now lists Stratis among supported backends, matching the issue request.
doc: revise README.md - simplify & move user guide content to docs/ #439 — Implements the README → Sphinx user guide split and Quick Start described by the issue.
doc: clarify list, create and show JSON report structure and keys #469 — Man page and user guide clarify JSON report structure and keys, aligning with this issue.

Possibly related PRs

doc: improve man2html compatibility in snapm.8 #434 — Edits the same man page; likely overlapping formatting/JSON examples.
doc: man page fixes & updates #400 — Overlaps on man-page JSON/reporting documentation and option naming.
doc: revise README.md - simplify & move user guide content to docs/ boom-boot#193 — Cross-repo documentation restructure that mirrors README→Sphinx changes.

Pre-merge checks (3 passed, 1 warning, 1 inconclusive)

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Out of Scope Changes Check	⚠️ Warning	While the documentation changes align with the linked issues, the modifications to CONTRIBUTING.md—such as expanded developer setup instructions and Sphinx build steps—are not covered by any of the specified linked issues (#237, #439, #469, #468) and therefore represent out-of-scope changes.	Consider extracting the CONTRIBUTING.md updates into a separate pull request or explicitly linking them to relevant documentation objectives to keep this PR focused on the issues at hand.
Linked Issues Check	❓ Inconclusive	The pull request clearly implements the Stratis inclusion in the README overview (#237), restructures and simplifies the README while moving detailed documentation to a Sphinx user guide with Quick Start, revised installation to `pip install .`, consistent terminology, and comprehensive CLI reference as required by #439, and documents the JSON schema and key naming for list, create, and show commands in both the user guide and man pages as specified in #469. However, it is unclear from the summaries whether all leading “# ” shell prompt markers have been removed from Markdown and RST files while preserving them in man pages to satisfy #468.	Please confirm that all command examples in Markdown and RST files have had leading “# ” removed as per issue #468, and that prompt markers remain only in the man pages.

✅ 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 “doc: Restructure documentation and add User Guide” concisely summarizes the primary change in this pull request, namely the overhaul of the documentation layout and the introduction of a dedicated user guide, making it clear and specific for anyone reviewing PR history.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

Poem

I nibble through the README bright,
I tuck docs snug before the night.
Conf files polished, guides all set,
New fields named — no worries yet.
A hop, a tweak — the burrow's right! 🥕

✨ Finishing Touches

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch bmr-update-readme-user-guide

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.}

packit-as-a-service · 2025-08-29T20:12:19Z

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

sudo yum install -y dnf-plugins-core on RHEL 8
sudo dnf install -y dnf-plugins-core on Fedora
dnf copr enable packit/snapshotmanager-snapm-454
And now you can install the packages.

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

coderabbitai

Actionable comments posted: 7

🧹 Nitpick comments (3)

README.md (3)
28-33: RPM install note: call out required boom version (Issue #401).

DNF can’t express semver constraints cleanly; add a note so users verify boom-boot >= 1.6.7.
 # Install from distribution repositories (when available)
 # dnf install snapm boom-boot
+#
+# Note: snapm ≥ 0.5.0 requires boom-boot ≥ 1.6.7 (LVM autodetection). Verify:
+#   boom --version
5-8: Minor style: trim “specific point in time”.

Concise phrasing reads better.
-to be captured atomically, representing the complete system state at a
-specific point in time.
+to be captured atomically, representing the complete system state at a point in time.
1-1: Third-party badge optional.

Consider moving “Ask DeepWiki” below the fold or to the docs site to keep the README vendor-neutral.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
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 932f679 and cc55c91.

📒 Files selected for processing (3)

README.md (1 hunks)
doc/index.rst (1 hunks)
doc/user_guide.rst (1 hunks)

🧰 Additional context used

🪛 LanguageTool

README.md

[grammar] ~6-~6: There might be a mistake here.
Context: ...apm allows snapshots of multiple volumes to be captured atomically, representing ...

(QB_NEW_EN)

[grammar] ~7-~7: There might be a mistake here.
Context: ...resenting the complete system state at a specific point in time. ## Features - ...

(QB_NEW_EN)

[style] ~8-~8: This phrase is redundant. Consider writing “point” or “time”.
Context: ...the complete system state at a specific point in time. ## Features - **Atomic snapshot sets...

(MOMENT_IN_TIME)

[grammar] ~109-~109: There might be a mistake here.
Context: ...** - Comprehensive usage documentation - **[API Documentation](https://snapm.readthe...

(QB_NEW_EN)

[grammar] ~111-~111: There might be a mistake here.
Context: ...edocs.io/)** - Auto-generated API docs - Manual Pages - System manual pa...

(QB_NEW_EN)

[grammar] ~116-~116: There might be a mistake here.
Context: ...ith LVM2, Stratis, or compatible storage - [boom-boot](https://github.com/snapshotma...

(QB_NEW_EN)

[grammar] ~118-~118: There might be a mistake here.
Context: ...otmanager/boom) for bootable snapshots - Python 3.8+ - Root privileges for snapsh...

(QB_NEW_EN)

[grammar] ~119-~119: There might be a mistake here.
Context: ...) for bootable snapshots - Python 3.8+ - Root privileges for snapshot operations ...

(QB_NEW_EN)

[grammar] ~124-~124: There might be a mistake here.
Context: ...Snapshots | Thin Provisioning | Status | |---------|------|-----------|----------...

(QB_NEW_EN)

[grammar] ~125-~125: There might be a mistake here.
Context: ...----------|-------------------|--------| | LVM2 | Copy-on-Write | ✓ | ✗ | Stable ...

(QB_NEW_EN)

[grammar] ~126-~126: There might be a mistake here.
Context: ... LVM2 | Copy-on-Write | ✓ | ✗ | Stable | | LVM2 | Thin Pools | ✓ | ✓ | Stable | |...

(QB_NEW_EN)

[grammar] ~127-~127: There might be a mistake here.
Context: ...| | LVM2 | Thin Pools | ✓ | ✓ | Stable | | Stratis | Thin Pools | ✓ | ✓ | Stable ...

(QB_NEW_EN)

[grammar] ~132-~132: There might be a mistake here.
Context: ...e see CONTRIBUTING.md for guidelines. ### Development Setup `...

(QB_NEW_EN)

[grammar] ~165-~165: There might be a mistake here.
Context: ...- Boot manager for Linux snapshot boot - [stratis-cli](https://github.com/stratis-...

(QB_NEW_EN)

[grammar] ~167-~167: There might be a mistake here.
Context: ... Command-line tool for Stratis storage - lvm2 -...

(QB_NEW_EN)

[grammar] ~168-~168: There might be a mistake here.
Context: ...team/lvm2) - LVM2 logical volume manager

(QB_NEW_EN)

⏰ 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). (24)

GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: virt_tests (bios, lvm-thin, fedora41)
GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
GitHub Check: virt_tests (bios, lvm, fedora41)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
GitHub Check: virt_tests (bios, lvm, fedora42)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
GitHub Check: virt_tests (uefi, lvm, fedora42)
GitHub Check: virt_tests (bios, lvm, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, fedora41)
GitHub Check: virt_tests (uefi, lvm, centos-stream9)
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm, centos-stream9)
GitHub Check: python-basic-tests

🔇 Additional comments (4)

doc/user_guide.rst (1)

148-179: Nice addition: autoindex docs and list fields.

Clear, consistent with list/reporting semantics.

doc/index.rst (1)

13-13: Toctree correctly points to the new user guide.

This aligns Sphinx nav with the README restructuring.
README.md (2)
124-129: Backend table is helpful and meets Issue #237 (Stratis listed).

Nice, clear at-a-glance comparison.

116-121: Align boom-boot minimum version with upstream release
Replace the unspecified boom-boot version in README.md, requirements.txt, and setup.cfg with the actual release that introduced LVM parameter autodetection (currently 1.6.6 on PyPI/GitHub). For example, in README.md:
- [boom-boot](https://github.com/snapshotmanager/boom) for bootable snapshots
+ [boom-boot](https://github.com/snapshotmanager/boom-boot) ≥ 1.6.6 for bootable snapshots (snapm ≥ 0.5.0 requires LVM parameter autodetection)
And in requirements.txt & setup.cfg:
boom-boot>=1.6.6
Then verify all references with:
rg -n 'boom-boot' -C2 README.md requirements.txt setup.cfg

bmr-cymru · 2025-08-29T20:23:00Z

virt_tests failures seem to be infra issues (all Fedora failed, all Centos Stream passed...).

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (5)

README.md (5)
5-8: Tighten wording; avoid redundant “moment/point in time” phrasing.

Streamline the intro for clarity.
-Snapshot manager (snapm) is a tool for managing coordinated sets of
-snapshots on Linux systems. Snapm allows snapshots of multiple volumes
-to be captured in sequence at a common moment in time, representing the
-complete system state at a specific point in time.
+Snapshot manager (snapm) manages coordinated sets of snapshots on Linux systems.
+It captures snapshots of multiple volumes in sequence so they represent the
+complete system state at a single point.
29-33: Make RPM example copy/paste‑friendly.

The code block currently shows commented commands. Present the actual command; the preceding “when available” caveat already protects expectations.
-# Install from distribution repositories (when available)
-# dnf install snapm boom-boot
+# Install from distribution repositories (when available)
+dnf install snapm boom-boot
34-44: Clarify boom-boot version expectations (Issue #401) without prematurely pinning.

Add a short note stating that snapm 0.5.0 will require boom-boot ≥ 1.6.7 for LVM parameter autodetection, but do not enforce it in commands until that release lands. This aligns with the plan while avoiding confusion right now.
 #### Manual Installation
 ```bash
 git clone https://github.com/snapshotmanager/boom-boot.git
 cd boom-boot
 pip install .
@@
 cd snapm
 pip install .
+> Note:
+> - snapm 0.5.0 will require boom-boot ≥ 1.6.7 due to LVM parameter autodetection.
+> - Until that boom-boot release is available, the source install above (or an
+> existing 1.6.x) is supported.
+> - If/when packages are published, users may prefer:
+> pip install boom-boot and pip install snapm.
Please confirm the exact wording you want here and whether we should link to the tracking issue for the autodetection feature.

---

`24-27`: **Explain the “root prompt (#)” convention once.**

Add a sentence so users know to copy only what comes after “#”.



```diff
 ## Quick Start
 
 ### Installation
+
+All shell examples use the root prompt (`#`). When copy/pasting, omit the leading `# `.
164-169: Minor wording polish in Related Projects.

Tighten description and align punctuation.
-- [boom](https://github.com/snapshotmanager/boom) - Boot manager for
-  Linux snapshot boot
+- [boom](https://github.com/snapshotmanager/boom) - Boot manager for Linux snapshots.
 - [stratis-cli](https://github.com/stratis-storage/stratis-cli) -
-  Command-line tool for Stratis storage
-- [lvm2](https://gitlab.com/lvmteam/lvm2) - LVM2 logical volume manager
+  Command-line tool for Stratis storage.
+- [lvm2](https://gitlab.com/lvmteam/lvm2) - LVM2 logical volume manager.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
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 cc55c91 and 5701b13.

📒 Files selected for processing (3)

README.md (1 hunks)
doc/index.rst (1 hunks)
doc/user_guide.rst (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

doc/index.rst
doc/user_guide.rst

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-29T20:21:30.040Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-44
Timestamp: 2025-08-29T20:21:30.040Z
Learning: boom-boot 1.6.7 is not yet released as of August 2025, so version requirements should not reference it until the actual release is made, even though snapm-0.5.0 is planned to depend on it.

Applied to files:

README.md

🪛 LanguageTool

README.md

[grammar] ~6-~6: There might be a mistake here.
Context: ...apm allows snapshots of multiple volumes to be captured in sequence at a common m...

(QB_NEW_EN)

[style] ~7-~7: This phrase is redundant. Consider writing “moment” or “time”.
Context: ... to be captured in sequence at a common moment in time, representing the complete system state...

(MOMENT_IN_TIME)

[grammar] ~7-~7: There might be a mistake here.
Context: ... common moment in time, representing the complete system state at a specific poin...

(QB_NEW_EN)

[style] ~8-~8: This phrase is redundant. Consider writing “point” or “time”.
Context: ...the complete system state at a specific point in time. ## Features - **Coordinated snapshot...

(MOMENT_IN_TIME)

[grammar] ~109-~109: There might be a mistake here.
Context: ...** - Comprehensive usage documentation - **[API Documentation](https://snapm.readthe...

(QB_NEW_EN)

[grammar] ~111-~111: There might be a mistake here.
Context: ...edocs.io/)** - Auto-generated API docs - Manual Pages - System manual pa...

(QB_NEW_EN)

[grammar] ~116-~116: There might be a mistake here.
Context: ...ith LVM2, Stratis, or compatible storage - [boom-boot](https://github.com/snapshotma...

(QB_NEW_EN)

[grammar] ~118-~118: There might be a mistake here.
Context: ...otmanager/boom) for bootable snapshots - Python 3.8+ - Root privileges for snapsh...

(QB_NEW_EN)

[grammar] ~119-~119: There might be a mistake here.
Context: ...) for bootable snapshots - Python 3.8+ - Root privileges for snapshot operations ...

(QB_NEW_EN)

[grammar] ~124-~124: There might be a mistake here.
Context: ...Snapshots | Thin Provisioning | Status | |---------|------|-----------|----------...

(QB_NEW_EN)

[grammar] ~125-~125: There might be a mistake here.
Context: ...----------|-------------------|--------| | LVM2 | Copy-on-Write | ✓ | ✗ | Stable ...

(QB_NEW_EN)

[grammar] ~126-~126: There might be a mistake here.
Context: ... LVM2 | Copy-on-Write | ✓ | ✗ | Stable | | LVM2 | Thin Pools | ✓ | ✓ | Stable | |...

(QB_NEW_EN)

[grammar] ~127-~127: There might be a mistake here.
Context: ...| | LVM2 | Thin Pools | ✓ | ✓ | Stable | | Stratis | Thin Pools | ✓ | ✓ | Stable ...

(QB_NEW_EN)

[grammar] ~132-~132: There might be a mistake here.
Context: ...e see CONTRIBUTING.md for guidelines. ### Development Setup `...

(QB_NEW_EN)

[grammar] ~165-~165: There might be a mistake here.
Context: ...- Boot manager for Linux snapshot boot - [stratis-cli](https://github.com/stratis-...

(QB_NEW_EN)

[grammar] ~167-~167: There might be a mistake here.
Context: ... Command-line tool for Stratis storage - lvm2 -...

(QB_NEW_EN)

[grammar] ~168-~168: There might be a mistake here.
Context: ...team/lvm2) - LVM2 logical volume manager

(QB_NEW_EN)

⏰ 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). (25)

GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: virt_tests (bios, lvm, centos-stream9)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, fedora41)
GitHub Check: virt_tests (uefi, lvm, fedora42)
GitHub Check: virt_tests (bios, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm, fedora42)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
GitHub Check: python-basic-tests

🔇 Additional comments (2)

README.md (2)

108-113: User Guide link looks correct now (doc/user_guide.rst).

This fixes the previous docs/ vs doc path issue. LGTM.

124-129: Stratis is clearly listed; objective #237 satisfied.

Backend table and Features both include Stratis. Nice.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (4)

README.md (4)
5-8: Tighten intro and explicitly name Stratis (Issue #237).

Clarify wording and satisfy the requirement to list Stratis in the intro.
-Snapshot manager (snapm) is a tool for managing coordinated sets of
-snapshots on Linux systems. Snapm allows snapshots of multiple volumes
-to be captured in sequence at a common moment in time, representing the
-complete system state at a specific point in time.
+Snapshot manager (snapm) manages coordinated sets of snapshots on Linux systems.
+It captures snapshots of multiple volumes in sequence at a common moment,
+representing the complete system state at a specific point.
+Supported backends: LVM2 (copy‑on‑write and thin‑provisioned) and Stratis.
26-27: Add a copy/paste note about the root prompt.

Examples use “#” as a root prompt; without a note, copy/paste will no-op because “# …” is a shell comment.
 ### Installation
+
+Note: Commands show a root prompt (#). When copying, omit the leading “#”.
36-44: Simplify boom-boot install (avoid cloning when PyPI suffices).

Prefer PyPI for users not hacking on boom-boot; keeps steps shorter and reproducible. (Per learning, don’t pin to unreleased 1.6.7.)
-git clone https://github.com/snapshotmanager/boom-boot.git
-cd boom-boot
-pip install .
+pip install boom-boot
@@
-# Clone and install snapm
+# Clone and install snapm (for development)
 git clone https://github.com/snapshotmanager/snapm.git
 cd snapm
 pip install .
108-111: Fix line-wrap nit in Documentation bullets.

Join the broken line for readability.
-- **[User Guide](doc/user_guide.rst)** - Comprehensive usage
-  documentation
+- **[User Guide](doc/user_guide.rst)** - Comprehensive usage documentation

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
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 5701b13 and 17dd959.

📒 Files selected for processing (3)

README.md (1 hunks)
doc/index.rst (1 hunks)
doc/user_guide.rst (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

doc/user_guide.rst
doc/index.rst

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-29T20:21:30.040Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-44
Timestamp: 2025-08-29T20:21:30.040Z
Learning: boom-boot 1.6.7 is not yet released as of August 2025, so version requirements should not reference it until the actual release is made, even though snapm-0.5.0 is planned to depend on it.

Applied to files:

README.md

🪛 LanguageTool

README.md

[grammar] ~6-~6: There might be a mistake here.
Context: ...apm allows snapshots of multiple volumes to be captured in sequence at a common m...

(QB_NEW_EN)

[style] ~7-~7: This phrase is redundant. Consider writing “moment” or “time”.
Context: ... to be captured in sequence at a common moment in time, representing the complete system state...

(MOMENT_IN_TIME)

[grammar] ~7-~7: There might be a mistake here.
Context: ... common moment in time, representing the complete system state at a specific poin...

(QB_NEW_EN)

[style] ~8-~8: This phrase is redundant. Consider writing “point” or “time”.
Context: ...the complete system state at a specific point in time. ## Features - **Coordinated snapshot...

(MOMENT_IN_TIME)

[grammar] ~109-~109: There might be a mistake here.
Context: ...** - Comprehensive usage documentation - **[API Documentation](https://snapm.readthe...

(QB_NEW_EN)

[grammar] ~111-~111: There might be a mistake here.
Context: ...edocs.io/)** - Auto-generated API docs - Manual Pages - System manual pa...

(QB_NEW_EN)

[grammar] ~116-~116: There might be a mistake here.
Context: ...ith LVM2, Stratis, or compatible storage - [boom-boot](https://github.com/snapshotma...

(QB_NEW_EN)

[grammar] ~118-~118: There might be a mistake here.
Context: ...ager/boom-boot) for bootable snapshots - Python 3.8+ - Root privileges for snapsh...

(QB_NEW_EN)

[grammar] ~119-~119: There might be a mistake here.
Context: ...) for bootable snapshots - Python 3.8+ - Root privileges for snapshot operations ...

(QB_NEW_EN)

[grammar] ~124-~124: There might be a mistake here.
Context: ...Snapshots | Thin Provisioning | Status | |---------|------|-----------|----------...

(QB_NEW_EN)

[grammar] ~125-~125: There might be a mistake here.
Context: ...----------|-------------------|--------| | LVM2 | Copy-on-Write | ✓ | ✗ | Stable ...

(QB_NEW_EN)

[grammar] ~126-~126: There might be a mistake here.
Context: ... LVM2 | Copy-on-Write | ✓ | ✗ | Stable | | LVM2 | Thin Pools | ✓ | ✓ | Stable | |...

(QB_NEW_EN)

[grammar] ~127-~127: There might be a mistake here.
Context: ...| | LVM2 | Thin Pools | ✓ | ✓ | Stable | | Stratis | Thin Pools | ✓ | ✓ | Stable ...

(QB_NEW_EN)

[grammar] ~132-~132: There might be a mistake here.
Context: ...e see CONTRIBUTING.md for guidelines. ### Development Setup `...

(QB_NEW_EN)

[grammar] ~165-~165: There might be a mistake here.
Context: ...- Boot manager for Linux snapshot boot - [stratis-cli](https://github.com/stratis-...

(QB_NEW_EN)

[grammar] ~167-~167: There might be a mistake here.
Context: ... Command-line tool for Stratis storage - lvm2 -...

(QB_NEW_EN)

[grammar] ~168-~168: There might be a mistake here.
Context: ...team/lvm2) - LVM2 logical volume manager

(QB_NEW_EN)

⏰ 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). (38)

GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
GitHub Check: virt_tests (uefi, lvm, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: virt_tests (bios, lvm, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
GitHub Check: virt_tests (uefi, lvm, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, centos-stream9)
GitHub Check: virt_tests (bios, lvm, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
GitHub Check: virt_tests (bios, lvm, centos-stream9)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
GitHub Check: virt_tests (bios, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm-thin, fedora41)
GitHub Check: python-basic-tests

🔇 Additional comments (6)

README.md (6)

48-52: Good: “snapshot set” in prose, “snapset” in CLI.

Terminology aligns with #439; example is clear.

116-121: Planned boom-boot version note for #401 (without pre-announcing unreleased 1.6.7).

Consider adding a brief “planned requirement” note (no hard pin yet) so readers know what’s coming in 0.5.0. If you prefer to defer until the release, skip.

Would you like me to add a short note under Requirements stating that snapm 0.5.0 will require boom-boot ≥1.6.7 (pending release), per #401, and update once it ships?

124-128: Backend table looks good.

Covers LVM2 CoW, LVM2 thin, and Stratis with clear status.

102-104: Verify CLI subcommand “snapshot revert”.

Double-check that the singular “snapshot revert” is correct for reverting a single volume; adjust if the actual subcommand differs.

164-166: Correct linkage and labeling for boom.

Link text and target repo now match. Nicely done.

1-169: Dependency note confirmed

Verified boom-boot>=1.6.0 is declared in requirements.txt (line 4) and setup.cfg (line 25).

bmr-cymru · 2025-08-29T20:45:35Z

@coderabbitai if they're installing snapm from source then they are probably either comfortable with installing boom from source too, or can figure out how to pip install themselves.

coderabbitai · 2025-08-29T20:45:58Z

Tip

For best results, initiate chat on the files or code changes.

@bmr-cymru That's a fair point! Users who are comfortable installing snapm from source are indeed likely to have the technical expertise to handle installing boom from source or using pip install as needed. This makes sense given that source installation typically requires familiarity with development workflows and dependency management.

Your approach of providing clear documentation while trusting users' technical capabilities seems appropriate for this audience. The updated installation instructions in the README strike a good balance between being helpful and not over-explaining for users who are already working with source installations.

🧠 Learnings used

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#383
File: virt_tests/testvm.py:493-499
Timestamp: 2025-08-26T15:22:27.918Z
Learning: In the snapm virt_tests testing workflow, the target distributions (Fedora 41/42, CentOS Stream 9/10) all provide the bare `pip` executable when python3-pip is installed, so using `pip install` instead of `python3 -m pip install` is acceptable and works reliably across these specific distributions.

bmr-cymru · 2025-08-29T20:47:19Z

@coderabbitai the line is broken to meet the 72-character line limit we enforce for source files. The linebreak is not rendered in MarkDown viewers.

coderabbitai · 2025-08-29T20:47:37Z

Tip

For best results, initiate chat on the files or code changes.

@bmr-cymru Thank you for the clarification! That makes perfect sense. The 72-character line limit is a good practice for source file readability, and you're absolutely right that Markdown viewers will render the broken lines seamlessly as continuous text. I appreciate you explaining the formatting standard - it's helpful to understand the intentional design choices behind the documentation structure.

✏️ Learnings added

Learnt from: bmr-cymru
URL: https://github.com/snapshotmanager/snapm/pull/454

Timestamp: 2025-08-29T20:47:36.535Z
Learning: The snapm project enforces a 72-character line limit for source files, including documentation files. Line breaks in Markdown files are intentionally used to meet this limit and will not be rendered as breaks in Markdown viewers.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#383
File: virt_tests/testvm.py:493-499
Timestamp: 2025-08-26T15:22:27.918Z
Learning: In the snapm virt_tests testing workflow, the target distributions (Fedora 41/42, CentOS Stream 9/10) all provide the bare `pip` executable when python3-pip is installed, so using `pip install` instead of `python3 -m pip install` is acceptable and works reliably across these specific distributions.

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T18:06:16.646Z
Learning: In the snapm virt_tests module, `unbuffer` is a hard requirement for CI and must be prepended to virt-install commands using `virt_install_cmd.insert(0, "unbuffer")`, not appended. The command should fail if unbuffer is not available since virt-install requires a controlling TTY.

bmr-cymru · 2025-08-29T21:30:42Z

@coderabbitai full review

coderabbitai · 2025-08-29T21:30:47Z

✅ Actions performed

Full review triggered.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (5)

doc/user_guide.rst (3)
657-659: Fix minor punctuation/formatting in “Reporting Commands” intro.

Add a comma between the listed commands and ensure consistent backtick groups for readability.
-The ``snapm snapset list``, ``snapm snapshot list`` ``snapm plugin
-list``, and ``snapm schedule list`` commands generate a tabular report
+The ``snapm snapset list``, ``snapm snapshot list``, ``snapm plugin
+list``, and ``snapm schedule list`` commands generate a tabular report
324-340: Make “help” field usage consistent (-o vs -o+).

Earlier you demonstrate -o+help, later the Field Reference shows -o help. Pick one form and use it consistently throughout to avoid confusion.

Would you like me to normalize all occurrences to -o help (or -o+help) in a follow-up patch?

Also applies to: 1241-1253

571-585: Align schedule create output label with snapset terminology.

Schedule output shows Boot: yes; elsewhere you use Bootable: yes. Consider standardizing the label name for consistency across commands, or add a brief note that schedule output uses Boot.
-   Boot: yes
+   Bootable: yes
README.md (2)
10-18: Normalize “copy‑on‑write” hyphenation.

Avoid mixed hyphen/nonbreaking-hyphen usage; stick to ASCII copy-on-write for consistency and to prevent odd rendering.
-Supported backends: LVM2 (copy‑on‑write and thin‑provisioned) and Stratis.
+Supported backends: LVM2 (copy-on-write and thin-provisioned) and Stratis.
@@
-- **Multiple backends**: Support for LVM2 (copy-on-write and thin),
+- **Multiple backends**: Support for LVM2 (copy-on-write and thin),
111-118: Consider linking Manual Pages to their canonical location.

(man/) may not resolve on GitHub; optionally link to RTD manpage HTML or note they’re available via man.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
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 932f679 and ab479bf.

📒 Files selected for processing (3)

README.md (1 hunks)
doc/index.rst (1 hunks)
doc/user_guide.rst (1 hunks)

🧰 Additional context used

🧠 Learnings (4)

📚 Learning: 2025-08-29T20:18:43.997Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: doc/user_guide.rst:518-530
Timestamp: 2025-08-29T20:18:43.997Z
Learning: For thin volumes (lvm2-thin provider), the "Free" field represents available space in the thin pool while "Size" represents the virtual size of the volume. It's normal and expected for Free > Size in thin provisioning scenarios.

Applied to files:

doc/user_guide.rst

📚 Learning: 2025-08-29T21:27:05.921Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.921Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

Applied to files:

README.md

📚 Learning: 2025-08-29T20:21:30.040Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-44
Timestamp: 2025-08-29T20:21:30.040Z
Learning: boom-boot 1.6.7 is not yet released as of August 2025, so version requirements should not reference it until the actual release is made, even though snapm-0.5.0 is planned to depend on it.

Applied to files:

README.md

📚 Learning: 2025-08-25T17:19:19.563Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

README.md

🪛 LanguageTool

README.md

[grammar] ~6-~6: There might be a mistake here.
Context: ...ots of multiple volumes in sequence at a common moment, representing the complete...

(QB_NEW_EN)

[grammar] ~7-~7: There might be a mistake here.
Context: ... the complete system state at a specific point in time. Supported backends: LVM2...

(QB_NEW_EN)

[style] ~7-~7: This phrase is redundant. Consider writing “point” or “time”.
Context: ...the complete system state at a specific point in time. Supported backends: LVM2 (copy‑on‑wri...

(MOMENT_IN_TIME)

[grammar] ~30-~30: There might be a mistake here.
Context: ...ompt (#). When copying, omit the leading “#”. #### From RPM (Fedora/CentOS/RHEL)...

(QB_NEW_EN)

[grammar] ~114-~114: There might be a mistake here.
Context: ...** - Comprehensive usage documentation - **[API Documentation](https://snapm.readthe...

(QB_NEW_EN)

[grammar] ~116-~116: There might be a mistake here.
Context: ...edocs.io/)** - Auto-generated API docs - Manual Pages - System manual pa...

(QB_NEW_EN)

[grammar] ~121-~121: There might be a mistake here.
Context: ...ith LVM2, Stratis, or compatible storage - [boom-boot](https://github.com/snapshotma...

(QB_NEW_EN)

[grammar] ~123-~123: There might be a mistake here.
Context: ...ager/boom-boot) for bootable snapshots - Python 3.8+ - Root privileges for snapsh...

(QB_NEW_EN)

[grammar] ~124-~124: There might be a mistake here.
Context: ...) for bootable snapshots - Python 3.8+ - Root privileges for snapshot operations ...

(QB_NEW_EN)

[grammar] ~129-~129: There might be a mistake here.
Context: ...Snapshots | Thin Provisioning | Status | |---------|------|-----------|----------...

(QB_NEW_EN)

[grammar] ~130-~130: There might be a mistake here.
Context: ...----------|-------------------|--------| | LVM2 | Copy-on-Write | ✓ | ✗ | Stable ...

(QB_NEW_EN)

[grammar] ~131-~131: There might be a mistake here.
Context: ... LVM2 | Copy-on-Write | ✓ | ✗ | Stable | | LVM2 | Thin Pools | ✓ | ✓ | Stable | |...

(QB_NEW_EN)

[grammar] ~132-~132: There might be a mistake here.
Context: ...| | LVM2 | Thin Pools | ✓ | ✓ | Stable | | Stratis | Thin Pools | ✓ | ✓ | Stable ...

(QB_NEW_EN)

[grammar] ~137-~137: There might be a mistake here.
Context: ...e see CONTRIBUTING.md for guidelines. ### Development Setup `...

(QB_NEW_EN)

[grammar] ~170-~170: There might be a mistake here.
Context: ...- Boot manager for Linux snapshot boot - [stratis-cli](https://github.com/stratis-...

(QB_NEW_EN)

[grammar] ~172-~172: There might be a mistake here.
Context: ... Command-line tool for Stratis storage - lvm2 -...

(QB_NEW_EN)

[grammar] ~173-~173: There might be a mistake here.
Context: ...team/lvm2) - LVM2 logical volume manager

(QB_NEW_EN)

⏰ 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). (28)

GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-42-x86_64

🔇 Additional comments (3)

doc/user_guide.rst (1)

518-530: Snapshot list example: Free > Size for thin volumes is correct.

The lvm2-thin row showing Free larger than Size is expected (thin pool free vs. virtual size). Keeping this example is good and clarifies semantics.

(Used prior learning: Free > Size is normal for lvm2-thin.)

doc/index.rst (2)

13-13: LGTM: toctree now points to user_guide.

Index cleanly routes users to the new guide.

13-15: Verify Sphinx build targets exist.

Confirm user_guide and modules targets resolve under the configured docs root (doc/ vs docs/), so RTD builds don’t break.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (5)

README.md (5)
5-8: Minor wording nit: simplify “point in time.”

“Specific point in time” is redundant; consider tightening.
- common moment, representing the complete system state at a specific
- point in time.
+ common moment, representing the complete system state at a specific time.
18-20: Parallelism nit in Features.

Make backend list parallel for readability.
-- **Multiple backends**: Support for LVM2 (copy-on-write and thin),
-  Stratis
+- **Multiple backends**: Support for LVM2 (copy-on-write and thin) and
+  Stratis
81-85: Clarify grub2-reboot usage.

Show a concrete example or placeholder for TITLE to reduce ambiguity.
-- Run ``grub2-reboot $TITLE`` to pre-select snapshot boot entry
+- Run ``grub2-reboot "$TITLE"`` to pre-select the snapshot boot entry,
+  e.g.:
+  ``grub2-reboot "Snapshot before-upgrade YYYY-MM-DD HH:MM:SS (version)"``
141-147: Use placeholders instead of real timestamp/UUID in sample output.

Static values can mislead; placeholders signal example output.
-Time:             2025-08-30 12:44:00
-UUID:             ee82269f-8c78-5814-b47a-b9be31bcebb5
+Time:             YYYY-MM-DD HH:MM:SS
+UUID:             01234567-89ab-cdef-0123-456789abcdef
203-204: Prefer module invocation for coverage + pytest.

Avoid hardcoding pytest path; this works across environments.
-# coverage run /usr/bin/pytest -v --log-level=debug tests/
+# coverage run -m pytest -v --log-level=debug tests/
+# coverage report -m

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
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 ab479bf and 27d07d5.

📒 Files selected for processing (3)

README.md (1 hunks)
doc/index.rst (1 hunks)
doc/user_guide.rst (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

doc/index.rst
doc/user_guide.rst

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-08-29T21:27:05.921Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.921Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

Applied to files:

README.md

📚 Learning: 2025-08-29T20:21:30.040Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-44
Timestamp: 2025-08-29T20:21:30.040Z
Learning: boom-boot 1.6.7 is not yet released as of August 2025, so version requirements should not reference it until the actual release is made, even though snapm-0.5.0 is planned to depend on it.

Applied to files:

README.md

📚 Learning: 2025-08-25T17:19:19.563Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

README.md

🪛 LanguageTool

README.md

[grammar] ~6-~6: There might be a mistake here.
Context: ...ots of multiple volumes in sequence at a common moment, representing the complete...

(QB_NEW_EN)

[grammar] ~7-~7: There might be a mistake here.
Context: ... the complete system state at a specific point in time. Supported backends: LVM2...

(QB_NEW_EN)

[style] ~7-~7: This phrase is redundant. Consider writing “point” or “time”.
Context: ...the complete system state at a specific point in time. Supported backends: LVM2 (copy‑on‑wri...

(MOMENT_IN_TIME)

[grammar] ~30-~30: There might be a mistake here.
Context: ...ompt (#). When copying, omit the leading “#”. #### From RPM (Fedora/CentOS/RHEL)...

(QB_NEW_EN)

[grammar] ~61-~61: There might be a mistake here.
Context: ...named "before-upgrade" of root and home, with snapshot boot and revert boot entri...

(QB_NEW_EN)

[grammar] ~81-~81: There might be a mistake here.
Context: ...boot and select the snapshot boot entry from GRUB menu - Entry will be named: "Snaps...

(QB_NEW_EN)

[grammar] ~84-~84: There might be a mistake here.
Context: ...)" - Run grub2-reboot $TITLE to pre-select snapshot boot entry #### Revert to sna...

(QB_NEW_EN)

[grammar] ~118-~118: There might be a mistake here.
Context: ...t revert pre-update When satisfied with update, clean up: bash # snapm snap...

(QB_NEW_EN)

[grammar] ~158-~158: There might be a mistake here.
Context: ...** - Comprehensive usage documentation - **[API Documentation](https://snapm.readthe...

(QB_NEW_EN)

[grammar] ~160-~160: There might be a mistake here.
Context: ...edocs.io/)** - Auto-generated API docs - Manual Pages - System manual pa...

(QB_NEW_EN)

[grammar] ~165-~165: There might be a mistake here.
Context: ...ith LVM2, Stratis, or compatible storage - [boom-boot](https://github.com/snapshotma...

(QB_NEW_EN)

[grammar] ~167-~167: There might be a mistake here.
Context: ...ager/boom-boot) for bootable snapshots - Python 3.8+ - Root privileges for snapsh...

(QB_NEW_EN)

[grammar] ~168-~168: There might be a mistake here.
Context: ...) for bootable snapshots - Python 3.8+ - Root privileges for snapshot operations ...

(QB_NEW_EN)

[grammar] ~173-~173: There might be a mistake here.
Context: ...Snapshots | Thin Provisioning | Status | |---------|------|-----------|----------...

(QB_NEW_EN)

[grammar] ~174-~174: There might be a mistake here.
Context: ...----------|-------------------|--------| | LVM2 | Copy-on-Write | ✓ | ✗ | Stable ...

(QB_NEW_EN)

[grammar] ~175-~175: There might be a mistake here.
Context: ... LVM2 | Copy-on-Write | ✓ | ✗ | Stable | | LVM2 | Thin Pools | ✓ | ✓ | Stable | |...

(QB_NEW_EN)

[grammar] ~176-~176: There might be a mistake here.
Context: ...| | LVM2 | Thin Pools | ✓ | ✓ | Stable | | Stratis | Thin Pools | ✓ | ✓ | Stable ...

(QB_NEW_EN)

[grammar] ~181-~181: There might be a mistake here.
Context: ...e see CONTRIBUTING.md for guidelines. ### Development Setup `...

(QB_NEW_EN)

[grammar] ~219-~219: There might be a mistake here.
Context: ...- Boot manager for Linux snapshot boot - [stratis-cli](https://github.com/stratis-...

(QB_NEW_EN)

[grammar] ~221-~221: There might be a mistake here.
Context: ... Command-line tool for Stratis storage - lvm2 -...

(QB_NEW_EN)

[grammar] ~222-~222: There might be a mistake here.
Context: ...team/lvm2) - LVM2 logical volume manager

(QB_NEW_EN)

⏰ 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). (41)

GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, fedora41)
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: virt_tests (uefi, lvm, fedora41)
GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
GitHub Check: virt_tests (bios, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm, centos-stream9)
GitHub Check: virt_tests (bios, lvm, fedora41)
GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
GitHub Check: virt_tests (bios, lvm, fedora42)
GitHub Check: virt_tests (bios, lvm, centos-stream9)
GitHub Check: python-basic-tests

🔇 Additional comments (4)

README.md (4)

5-11: README intro and backend list look good; Stratis addition meets #237.

Clear overview with Stratis explicitly listed. This aligns with the PR objectives.

41-55: Manual install steps are appropriate given boom-boot release timing.

Acknowledging prior learning about deferring a minimum boom-boot version until release; cloning is fine for now. No change requested.

157-161: Docs links LGTM.

User Guide path and external docs links resolve and match repo structure.

171-178: Backend table reads well.

Clear and succinct capability summary.

coderabbitai

Actionable comments posted: 0

♻️ Duplicate comments (1)

README.md (1)

42-54: Optional: offer PyPI install as an alternative path

If acceptable, add a short note that users may also “pip install boom-boot” and “pip install snapm” when available, keeping the clone flow for source installs. Avoid pinning to unreleased versions (per PR scope).

🧹 Nitpick comments (4)

README.md (4)
167-171: Clarify boom-boot is only needed for bootable snapshots

Avoid implying it’s a hard requirement for non-boot workflows.
 - [boom-boot](https://github.com/snapshotmanager/boom-boot)
+ - [boom-boot](https://github.com/snapshotmanager/boom-boot) (for bootable snapshots)
197-205: Use coverage with -m to avoid hardcoded pytest path

This is more portable across environments.
-# coverage run /usr/bin/pytest -v --log-level=debug tests/
+# coverage run -m pytest -v --log-level=debug tests/
81-87: Minor grammar: “the GRUB menu”

Keeps wording tight; no behavioral change.
-- Reboot and select the snapshot boot entry from GRUB menu
+- Reboot and select the snapshot boot entry from the GRUB menu
29-31: Use backticks around # to avoid smart-quote copy issues

Improves copy/paste experience.
-Note: Commands show a root prompt (#). When copying, omit the leading
-“#”.
+Note: Commands show a root prompt (`#`). When copying, omit the leading
+`#`.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
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 27d07d5 and dd38256.

📒 Files selected for processing (3)

README.md (1 hunks)
doc/index.rst (1 hunks)
doc/user_guide.rst (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

doc/user_guide.rst
doc/index.rst

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-08-29T21:27:05.921Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.921Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

Applied to files:

README.md

📚 Learning: 2025-08-29T20:21:30.040Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-44
Timestamp: 2025-08-29T20:21:30.040Z
Learning: boom-boot 1.6.7 is not yet released as of August 2025, so version requirements should not reference it until the actual release is made, even though snapm-0.5.0 is planned to depend on it.

Applied to files:

README.md

📚 Learning: 2025-08-25T17:19:19.563Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

README.md

🪛 LanguageTool

README.md

[grammar] ~6-~6: There might be a mistake here.
Context: ...ots of multiple volumes in sequence at a common moment, representing the complete...

(QB_NEW_EN)

[grammar] ~29-~29: There might be a mistake here.
Context: ...ompt (#). When copying, omit the leading “#”. #### From RPM (Fedora/CentOS/RHEL)...

(QB_NEW_EN)

[grammar] ~60-~60: There might be a mistake here.
Context: ...named "before-upgrade" of root and home, with snapshot boot and revert boot entri...

(QB_NEW_EN)

[grammar] ~81-~81: There might be a mistake here.
Context: ...boot and select the snapshot boot entry from GRUB menu - Entry will be named: "Snaps...

(QB_NEW_EN)

[grammar] ~120-~120: There might be a mistake here.
Context: ...t revert pre-update When satisfied with update, clean up: bash # snapm snap...

(QB_NEW_EN)

[grammar] ~160-~160: There might be a mistake here.
Context: ...** - Comprehensive usage documentation - **[API Documentation](https://snapm.readthe...

(QB_NEW_EN)

[grammar] ~162-~162: There might be a mistake here.
Context: ...edocs.io/)** - Auto-generated API docs - Manual Pages - System manual pa...

(QB_NEW_EN)

[grammar] ~167-~167: There might be a mistake here.
Context: ...inux system with LVM2 or Stratis volumes - [boom-boot](https://github.com/snapshotma...

(QB_NEW_EN)

[grammar] ~168-~168: There might be a mistake here.
Context: ...ith LVM2 or Stratis volumes - boom-boot - Root privileges for performing storage o...

(QB_NEW_EN)

[grammar] ~169-~169: There might be a mistake here.
Context: ...ileges for performing storage operations - Python 3.9+ ## Supported Storage Backen...

(QB_NEW_EN)

[grammar] ~174-~174: There might be a mistake here.
Context: ...Snapshots | Thin Provisioning | Status | |---------|------|-----------|----------...

(QB_NEW_EN)

[grammar] ~175-~175: There might be a mistake here.
Context: ...----------|-------------------|--------| | LVM2 | Copy-on-Write | ✓ | ✗ | Stable ...

(QB_NEW_EN)

[grammar] ~176-~176: There might be a mistake here.
Context: ... LVM2 | Copy-on-Write | ✓ | ✗ | Stable | | LVM2 | Thin Pools | ✓ | ✓ | Stable | |...

(QB_NEW_EN)

[grammar] ~177-~177: There might be a mistake here.
Context: ...| | LVM2 | Thin Pools | ✓ | ✓ | Stable | | Stratis | Thin Pools | ✓ | ✓ | Stable ...

(QB_NEW_EN)

[grammar] ~182-~182: There might be a mistake here.
Context: ...e see CONTRIBUTING.md for guidelines. ### Development Setup `...

(QB_NEW_EN)

[grammar] ~220-~220: There might be a mistake here.
Context: ...- Boot manager for Linux snapshot boot - [stratis-cli](https://github.com/stratis-...

(QB_NEW_EN)

[grammar] ~222-~222: There might be a mistake here.
Context: ... Command-line tool for Stratis storage - lvm2 -...

(QB_NEW_EN)

[grammar] ~223-~223: There might be a mistake here.
Context: ...team/lvm2) - LVM2 logical volume manager

(QB_NEW_EN)

⏰ 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). (27)

GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: container-tests
GitHub Check: python-basic-tests

🔇 Additional comments (1)

README.md (1)

159-164: In-repo links validated – doc/user_guide.rst and man/ both exist and are correctly linked from README.md.

bmr-cymru · 2025-08-30T13:45:28Z

@coderabbitai as now written, boom is a hard requirement. It's imported by snapm.manager._boot which is imported and used by Manager.

coderabbitai · 2025-08-30T13:46:18Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the import structure mentioned by bmr-cymru
echo "=== Checking snapm.manager._boot imports ==="
ast-grep --pattern 'import boom'

echo -e "\n=== Checking Manager class and its usage of _boot ==="
ast-grep --pattern 'from snapm.manager._boot import $_'

echo -e "\n=== Looking for _boot module ==="
fd "_boot.py" --type f

echo -e "\n=== Checking Manager class definition and _boot usage ==="
rg -n "class Manager" --type py -A 10

echo -e "\n=== Checking boom imports in snapm/manager/ ==="
rg -n "import boom" --type py snapm/manager/ || echo "No boom imports found in snapm/manager/"
rg -n "from boom" --type py snapm/manager/ || echo "No 'from boom' imports found in snapm/manager/"

Length of output: 3885

🏁 Script executed:

#!/bin/bash
# Check imports in the Manager class file
echo "=== Checking imports in snapm/manager/_manager.py ==="
head -50 snapm/manager/_manager.py | grep -E "^(import|from)"

echo -e "\n=== Checking for _boot usage in Manager ==="
rg -n "_boot" snapm/manager/_manager.py

echo -e "\n=== Checking snapm/manager/__init__.py for _boot imports ==="
cat snapm/manager/__init__.py | grep -E "(import|from).*_boot"

echo -e "\n=== Looking for any references to boot functionality in Manager ==="
rg -n -i "boot" snapm/manager/_manager.py | head -10

Length of output: 2950

@bmr-cymru You're absolutely correct! I can see from the codebase that boom is indeed a hard requirement. The Manager class in snapm/manager/_manager.py directly imports from ._boot:

from ._boot import (
    BootCache,
    create_snapset_boot_entry,
    delete_snapset_boot_entry,

And the Manager class extensively uses boot functionality - it initializes a _boot_cache, calls create_snapset_boot_entry(), delete_snapset_boot_entry(), and has methods like create_snapshot_set_boot_entry(). Since the _boot module imports boom modules directly (import boom, import boom.cache, import boom.command, etc.), boom is required at import time when the Manager class is instantiated.

This means the installation documentation correctly treats boom as a dependency that needs to be installed alongside snapm. Thank you for the clarification - this is important for understanding the architecture and dependency requirements.

🐰✨

✏️ Learnings added

Learnt from: bmr-cymru
URL: https://github.com/snapshotmanager/snapm/pull/454

Timestamp: 2025-08-30T13:46:15.289Z
Learning: In snapm, boom is a hard requirement because snapm.manager._boot imports boom modules directly, and the Manager class imports and uses functionality from _boot including BootCache, create_snapset_boot_entry, and delete_snapset_boot_entry. The Manager class cannot be instantiated without boom being available.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T18:06:16.646Z
Learning: In the snapm virt_tests module, `unbuffer` is a hard requirement for CI and must be prepended to virt-install commands using `virt_install_cmd.insert(0, "unbuffer")`, not appended. The command should fail if unbuffer is not available since virt-install requires a controlling TTY.

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#383
File: virt_tests/testvm.py:493-499
Timestamp: 2025-08-26T15:22:27.918Z
Learning: In the snapm virt_tests testing workflow, the target distributions (Fedora 41/42, CentOS Stream 9/10) all provide the bare `pip` executable when python3-pip is installed, so using `pip install` instead of `python3 -m pip install` is acceptable and works reliably across these specific distributions.

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T19:20:21.720Z
Learning: In the snapm virt_tests workflow, optimizing Git checkouts by using virtiofs mapping is complex because it requires checkouts for both `snapm` and `boom` repositories. Git checkouts aren't the dominant time cost, so the current approach of cloning within VMs is more practical than adding workflow complexity and local user setup requirements.

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#383
File: virt_tests/testvm.py:284-296
Timestamp: 2025-08-26T03:42:26.409Z
Learning: In the snapm virt_tests module, the current virt-install --boot configuration using "uefi=on/off" syntax works correctly across all target platforms that bmr-cymru tests on, so compatibility changes to avoid "uefi=off" are not needed.

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.921Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-44
Timestamp: 2025-08-29T20:21:30.040Z
Learning: boom-boot 1.6.7 is not yet released as of August 2025, so version requirements should not reference it until the actual release is made, even though snapm-0.5.0 is planned to depend on it.

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (6)

README.md (6)

5-7: Tighten “sequence at a common moment” phrasing

“in sequence” vs “common moment” reads contradictory. Suggest clearer, single-point-in-time wording.

-Snapshot manager (snapm) manages coordinated sets of snapshots on Linux
-systems.  It captures snapshots of multiple volumes in sequence at a
-common moment, representing the complete system state at a specific time.
+Snapshot manager (snapm) manages coordinated sets of snapshots on Linux
+systems. It coordinates snapshots of multiple volumes to capture a
+consistent point‑in‑time system state.

9-9: Remove duplication with the “Supported Storage Backends” section

This standalone line is redundant with the detailed table below.

-Supported backends: LVM2 (copy‑on‑write and thin‑provisioned) and Stratis.

42-54: Prefer PyPI install first, keep source as fallback

This shortens onboarding; keep clone path for contributors.

-Clone ``boom-boot`` and ``snapm`` repositories and install using pip:
+Install from PyPI:
+
+```bash
+# pip install boom-boot snapm
+```
+
+Or install from source:
 
 ```bash
 # git clone https://github.com/snapshotmanager/boom-boot.git
 # cd boom-boot
 # pip install .

# git clone https://github.com/snapshotmanager/snapm.git
# cd snapm
# pip install .


---

`167-171`: **Clarify bootable requirements**

Make explicit that bootable snapshots need supported backend for / and /boot.

```diff
 - Linux system with LVM2 or Stratis volumes
 - [boom-boot](https://github.com/snapshotmanager/boom-boot)
 - Root privileges for performing storage operations
 - Python 3.9+
+  - For bootable snapshots: the root filesystem (and /boot if separate)
+    must reside on a supported backend and use GRUB2.

204-205: Use module invocation for coverage+pytest

Portable across environments.

-# coverage run /usr/bin/pytest -v --log-level=debug tests/
+# coverage run -m pytest -v --log-level=debug tests/

219-224: Add boom-boot to Related Projects and clarify roles

Readers see both components and their purpose.

 - [boom](https://github.com/snapshotmanager/boom) - Boot manager for
   Linux snapshot boot
+- [boom-boot](https://github.com/snapshotmanager/boom-boot) - Boot entry
+  integration used by Snapm
 - [stratis-cli](https://github.com/stratis-storage/stratis-cli) -
   Command-line tool for Stratis storage
 - [lvm2](https://gitlab.com/lvmteam/lvm2) - LVM2 logical volume manager

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
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 dd38256 and b380fff.

📒 Files selected for processing (3)

README.md (1 hunks)
doc/index.rst (1 hunks)
doc/user_guide.rst (1 hunks)

✅ Files skipped from review due to trivial changes (1)

doc/user_guide.rst

🚧 Files skipped from review as they are similar to previous changes (1)

doc/index.rst

🧰 Additional context used

🧠 Learnings (5)

📓 Common learnings

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.921Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

📚 Learning: 2025-08-30T13:46:15.289Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-30T13:46:15.289Z
Learning: In snapm, boom is a hard requirement because snapm.manager._boot imports boom modules directly, and the Manager class imports and uses functionality from _boot including BootCache, create_snapset_boot_entry, and delete_snapset_boot_entry. The Manager class cannot be instantiated without boom being available.

Applied to files:

README.md

📚 Learning: 2025-08-29T21:27:05.921Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.921Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

Applied to files:

README.md

📚 Learning: 2025-08-29T20:21:30.040Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-44
Timestamp: 2025-08-29T20:21:30.040Z
Learning: boom-boot 1.6.7 is not yet released as of August 2025, so version requirements should not reference it until the actual release is made, even though snapm-0.5.0 is planned to depend on it.

Applied to files:

README.md

📚 Learning: 2025-08-25T17:19:19.563Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

README.md

🪛 LanguageTool

README.md

[grammar] ~6-~6: There might be a mistake here.
Context: ...ots of multiple volumes in sequence at a common moment, representing the complete...

(QB_NEW_EN)

[grammar] ~29-~29: There might be a mistake here.
Context: ...pt (#). When copying, omit the leading #. #### From RPM (Fedora/CentOS/RHEL)...

(QB_NEW_EN)

[grammar] ~60-~60: There might be a mistake here.
Context: ...named "before-upgrade" of root and home, with snapshot boot and revert boot entri...

(QB_NEW_EN)

[grammar] ~120-~120: There might be a mistake here.
Context: ...t revert pre-update When satisfied with update, clean up: bash # snapm snap...

(QB_NEW_EN)

[grammar] ~160-~160: There might be a mistake here.
Context: ...** - Comprehensive usage documentation - **[API Documentation](https://snapm.readthe...

(QB_NEW_EN)

[grammar] ~162-~162: There might be a mistake here.
Context: ...edocs.io/)** - Auto-generated API docs - Manual Pages - System manual pa...

(QB_NEW_EN)

[grammar] ~167-~167: There might be a mistake here.
Context: ...inux system with LVM2 or Stratis volumes - [boom-boot](https://github.com/snapshotma...

(QB_NEW_EN)

[grammar] ~168-~168: There might be a mistake here.
Context: ...ith LVM2 or Stratis volumes - boom-boot - Root privileges for performing storage o...

(QB_NEW_EN)

[grammar] ~169-~169: There might be a mistake here.
Context: ...ileges for performing storage operations - Python 3.9+ ## Supported Storage Backen...

(QB_NEW_EN)

[grammar] ~174-~174: There might be a mistake here.
Context: ...Snapshots | Thin Provisioning | Status | |---------|------|-----------|----------...

(QB_NEW_EN)

[grammar] ~175-~175: There might be a mistake here.
Context: ...----------|-------------------|--------| | LVM2 | Copy-on-Write | ✓ | ✗ | Stable ...

(QB_NEW_EN)

[grammar] ~176-~176: There might be a mistake here.
Context: ... LVM2 | Copy-on-Write | ✓ | ✗ | Stable | | LVM2 | Thin Pools | ✓ | ✓ | Stable | |...

(QB_NEW_EN)

[grammar] ~177-~177: There might be a mistake here.
Context: ...| | LVM2 | Thin Pools | ✓ | ✓ | Stable | | Stratis | Thin Pools | ✓ | ✓ | Stable ...

(QB_NEW_EN)

[grammar] ~182-~182: There might be a mistake here.
Context: ...e see CONTRIBUTING.md for guidelines. ### Development Setup `...

(QB_NEW_EN)

[grammar] ~220-~220: There might be a mistake here.
Context: ...- Boot manager for Linux snapshot boot - [stratis-cli](https://github.com/stratis-...

(QB_NEW_EN)

[grammar] ~222-~222: There might be a mistake here.
Context: ... Command-line tool for Stratis storage - lvm2 -...

(QB_NEW_EN)

[grammar] ~223-~223: There might be a mistake here.
Context: ...team/lvm2) - LVM2 logical volume manager

(QB_NEW_EN)

⏰ 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). (45)

GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
GitHub Check: virt_tests (bios, lvm, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
GitHub Check: virt_tests (uefi, lvm, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: virt_tests (bios, lvm-thin, fedora41)
GitHub Check: virt_tests (uefi, lvm, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
GitHub Check: virt_tests (bios, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
GitHub Check: virt_tests (bios, lvm, fedora42)
GitHub Check: python-basic-tests

🔇 Additional comments (2)

README.md (2)

1-224: Overall: README restructure hits the PR goals

Stratis is called out, Quick Start + docs split looks good, terminology aligns (“snapshot set” vs “snapset”). Nice job.

159-164: Specify the exact Read the Docs URL for your Snapm project (e.g., https://snapm.readthedocs.io/en/latest/) so I can verify whether the User Guide is served at user_guide.html or under a user_guide/ directory.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (7)

README.md (7)
1-1: Consider removing third-party badge or moving it below the title.
External “Ask DeepWiki” badges can distract; many projects keep only status/license badges in the header.

5-7: Tighten wording: “in sequence at a common moment.”
The phrase is contradictory. Suggest a clearer, single‑point‑in‑time consistency statement.
-Snapshot manager (snapm) manages coordinated sets of snapshots on Linux
-systems.  It captures snapshots of multiple volumes in sequence at a
-common moment, representing the complete system state at a specific time.
+Snapshot manager (snapm) manages coordinated sets of snapshots on Linux
+systems. It orchestrates snapshots across multiple volumes to capture
+a consistent point‑in‑time system state.
9-9: Standardize hyphenation for “copy-on-write.”
Line 9 uses a Unicode non-breaking hyphen; line 17 uses ASCII. Use one form (ASCII recommended) for searchability.
-Supported backends: LVM2 (copy‑on‑write and thin‑provisioned) and Stratis.
+Supported backends: LVM2 (copy-on-write and thin-provisioned) and Stratis.
Also applies to: 17-17

42-54: Prefer python3 -m pip for clarity; optional virtualenv note.
Avoids PATH/Python mismatch and is copy/paste safe.
-# git clone https://github.com/snapshotmanager/boom-boot.git
-# cd boom-boot
-# pip install .
+# git clone https://github.com/snapshotmanager/boom-boot.git
+# cd boom-boot
+# python3 -m pip install .
@@
-# git clone https://github.com/snapshotmanager/snapm.git
-# cd snapm
-# pip install .
+# git clone https://github.com/snapshotmanager/snapm.git
+# cd snapm
+# python3 -m pip install .
Optional: add a one-liner above these blocks suggesting a venv for manual installs.

136-150: Output block is still fenced as bash.
Mark the example output as plain code to prevent accidental execution when copied.
-Example output:
-```bash
+Example output:
+```
 SnapsetName:      dev-checkpoint-home
 Sources:          /home
 NrSnapshots:      1
 Time:             2025-08-30 12:44:00
 UUID:             ee82269f-8c78-5814-b47a-b9be31bcebb5
 Status:           Inactive
 Autoactivate:     no
 Bootable:         no
---

`196-201`: **Use python3 -m pip in dev setup.**
Keeps interpreter consistent with python_requires and avoids PATH surprises.

```diff
-# pip install -e .
-# pip install -r requirements-dev.txt
+# python3 -m pip install -e .
+# python3 -m pip install -r requirements-dev.txt
214-215: Avoid hard-coded pytest path in coverage example.
Use module invocation so it works across environments.
-# coverage run /usr/bin/pytest -v --log-level=debug tests/
+# python3 -m coverage run -m pytest -v --log-level=debug tests/

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
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 b380fff and de8af90.

📒 Files selected for processing (3)

README.md (1 hunks)
doc/index.rst (1 hunks)
doc/user_guide.rst (1 hunks)

✅ Files skipped from review due to trivial changes (1)

doc/index.rst

🚧 Files skipped from review as they are similar to previous changes (1)

doc/user_guide.rst

🧰 Additional context used

🧠 Learnings (7)

📓 Common learnings

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.921Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

📚 Learning: 2025-08-30T13:46:15.289Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-30T13:46:15.289Z
Learning: In snapm, boom is a hard requirement because snapm.manager._boot imports boom modules directly, and the Manager class imports and uses functionality from _boot including BootCache, create_snapset_boot_entry, and delete_snapset_boot_entry. The Manager class cannot be instantiated without boom being available.

Applied to files:

README.md

📚 Learning: 2025-08-29T21:27:05.921Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.921Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

Applied to files:

README.md

📚 Learning: 2025-08-30T14:13:38.688Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-38
Timestamp: 2025-08-30T14:13:38.688Z
Learning: The boom-boot package structure: `boom-boot` is the leaf package containing the binaries in all distributions, while `python3-boom` contains only the Python modules. For installation instructions, `boom-boot` is the correct package name to recommend.

Applied to files:

README.md

📚 Learning: 2025-08-29T20:21:30.040Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-44
Timestamp: 2025-08-29T20:21:30.040Z
Learning: boom-boot 1.6.7 is not yet released as of August 2025, so version requirements should not reference it until the actual release is made, even though snapm-0.5.0 is planned to depend on it.

Applied to files:

README.md

📚 Learning: 2025-08-25T17:19:19.563Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

README.md

📚 Learning: 2025-08-23T22:16:04.019Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:11-12
Timestamp: 2025-08-23T22:16:04.019Z
Learning: In man pages for the snapm project, prefer backslash (`\`) continuations over multiple macro invocations when breaking long command lists across lines, as this maintains a single macro invocation while being readable in 80x25 terminals.

Applied to files:

README.md

🪛 LanguageTool

README.md

[grammar] ~6-~6: There might be a mistake here.
Context: ...ots of multiple volumes in sequence at a common moment, representing the complete...

(QB_NEW_EN)

[grammar] ~29-~29: There might be a mistake here.
Context: ...pt (#). When copying, omit the leading #. #### From RPM (Fedora/CentOS/RHEL)...

(QB_NEW_EN)

[grammar] ~60-~60: There might be a mistake here.
Context: ...named "before-upgrade" of root and home, with snapshot boot and revert boot entri...

(QB_NEW_EN)

[grammar] ~120-~120: There might be a mistake here.
Context: ...t revert pre-update When satisfied with update, clean up: bash # snapm snap...

(QB_NEW_EN)

[grammar] ~170-~170: There might be a mistake here.
Context: ...** - Comprehensive usage documentation - **[API Documentation](https://snapm.readthe...

(QB_NEW_EN)

[grammar] ~172-~172: There might be a mistake here.
Context: ...edocs.io/)** - Auto-generated API docs - Manual Pages - System manual pa...

(QB_NEW_EN)

[grammar] ~177-~177: There might be a mistake here.
Context: ...inux system with LVM2 or Stratis volumes - [boom-boot](https://github.com/snapshotma...

(QB_NEW_EN)

[grammar] ~178-~178: There might be a mistake here.
Context: ...ith LVM2 or Stratis volumes - boom-boot - Root privileges for performing storage o...

(QB_NEW_EN)

[grammar] ~179-~179: There might be a mistake here.
Context: ...ileges for performing storage operations - Python 3.9+ ## Supported Storage Backen...

(QB_NEW_EN)

[grammar] ~184-~184: There might be a mistake here.
Context: ...Snapshots | Thin Provisioning | Status | |---------|------|-----------|----------...

(QB_NEW_EN)

[grammar] ~185-~185: There might be a mistake here.
Context: ...----------|-------------------|--------| | LVM2 | Copy-on-Write | ✓ | ✗ | Stable ...

(QB_NEW_EN)

[grammar] ~186-~186: There might be a mistake here.
Context: ... LVM2 | Copy-on-Write | ✓ | ✗ | Stable | | LVM2 | Thin Pools | ✓ | ✓ | Stable | |...

(QB_NEW_EN)

[grammar] ~187-~187: There might be a mistake here.
Context: ...| | LVM2 | Thin Pools | ✓ | ✓ | Stable | | Stratis | Thin Pools | ✓ | ✓ | Stable ...

(QB_NEW_EN)

[grammar] ~192-~192: There might be a mistake here.
Context: ...e see CONTRIBUTING.md for guidelines. ### Development Setup `...

(QB_NEW_EN)

[grammar] ~230-~230: There might be a mistake here.
Context: ...- Boot manager for Linux snapshot boot - [stratis-cli](https://github.com/stratis-...

(QB_NEW_EN)

[grammar] ~232-~232: There might be a mistake here.
Context: ... Command-line tool for Stratis storage - lvm2 -...

(QB_NEW_EN)

[grammar] ~233-~233: There might be a mistake here.
Context: ...team/lvm2) - LVM2 logical volume manager

(QB_NEW_EN)

⏰ 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). (45)

GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: virt_tests (bios, lvm, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm, fedora42)
GitHub Check: virt_tests (uefi, lvm, fedora41)
GitHub Check: virt_tests (uefi, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, fedora41)
GitHub Check: virt_tests (bios, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
GitHub Check: virt_tests (bios, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm, centos-stream9)
GitHub Check: virt_tests (uefi, lvm, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
GitHub Check: python-basic-tests

🔇 Additional comments (4)

README.md (4)

13-23: Verify terminology alignment and plugin docs cross-reference.
README says “Plugin architecture”; ensure doc/user_guide.rst includes a section describing plugin backends and that prose consistently uses “snapshot set” (text) vs “snapset” (CLI).

36-38: RPM install line looks good (boom-boot is correct).
This matches distro packaging where boom-boot provides binaries; thanks for keeping it accurate.

175-181: Requirements section looks accurate (boom-boot + Python 3.9+).
Matches current packaging constraints and prior discussions.

167-174: Documentation links valid
Confirmed that doc/user_guide.rst exists and man/ contains the referenced pages.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (4)

man/man8/snapm.8 (4)
79-88: Clarify mutual exclusivity of --rows vs --json in ARGS_REPORT_OPTS.

The current “RB [ --rows | --json ]” reads as an exclusive choice; if that’s intended, good. If not, split them to avoid implying exclusivity.

Apply if not exclusive:
- .  RB [ --rows | --json ] " "\c
+ .  RB [ --rows ] " "\c
+ .  RB [ --json ] " "\c
629-636: Consider stating underscore policy explicitly.

You list “a–z, A–Z, 0–9, + . -” as valid. If “_” (underscore) is disallowed, say so; if allowed, include it to prevent guesswork.

667-671: Define status precedence for mixed member states.

You specify Inactive and Invalid aggregation, but not how Reverting interacts (e.g., does Reverting override Invalid/Inactive?). Add a short precedence rule to avoid ambiguity.

1468-1496: Document that JSON output from show subcommands is always an array of objects
Add a note in the man page immediately following the example to clarify that --json for all “show” commands emits a JSON array—even when only a single item matches.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e942689 and f4a9ebd.

📒 Files selected for processing (2)

doc/user_guide.rst (1 hunks)
man/man8/snapm.8 (21 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

doc/user_guide.rst

🧰 Additional context used

🧠 Learnings (11)

📓 Common learnings

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

📚 Learning: 2025-08-24T16:09:15.238Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:496-505
Timestamp: 2025-08-24T16:09:15.238Z
Learning: In UNIX man pages for snapm, metavars (value placeholders) should be lowercase except for UUID which should be uppercase. For example, use `schedule_name` not `SCHEDULE_NAME`, but use `UUID` not `uuid`.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T19:34:08.396Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-22T19:34:08.396Z
Learning: The snapshotmanager organization (including snapm and boom projects) uses "SHORTMONTH DAY YEAR" format for man page headers. This format has been used by boom since 2017. Organizational consistency within snapshotmanager is prioritized over cross-project consistency when it comes to formatting standards.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T18:27:31.307Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-22T18:27:31.307Z
Learning: The snapm project only supports GNU Groff markup for man pages and does not support other troff toolchains. All man page formatting should follow GNU Groff standards and practices.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-23T22:16:04.019Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:11-12
Timestamp: 2025-08-23T22:16:04.019Z
Learning: In man pages for the snapm project, prefer backslash (`\`) continuations over multiple macro invocations when breaking long command lists across lines, as this maintains a single macro invocation while being readable in 80x25 terminals.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-09-09T19:26:01.082Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1434-1462
Timestamp: 2025-09-09T19:26:01.082Z
Learning: The `list` command JSON format in snapm uses a wrapper object structure with a type-specific key (e.g., "Snapsets", "Snapshots") that maps to an array of objects, rather than a direct JSON array. This design provides flexibility for future mixed-object-type reports and follows patterns similar to lvm2's JSON reports. For example: `{"Snapsets": [{"name": "backup", ...}, {"name": "before-upgrade", ...}]}`.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-09-09T16:06:19.848Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-20T21:09:39.353Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-20T21:09:39.353Z
Learning: In the snapm codebase, JSON output formatting follows a pattern: use `to_dict()+dumps()` for operations returning multiple objects (like list commands) to properly construct JSON arrays, and use `object.json()` for operations returning single objects (like create commands). This avoids the complexity of assembling multiple JSON object strings into valid JSON lists.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-25T17:19:19.563Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-25T01:33:17.496Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#434
File: man/man8/snapm.8:15-26
Timestamp: 2025-08-25T01:33:17.496Z
Learning: For man page URL macro fallbacks in the snapm project, man2html compatibility requires simple macro definitions. Complex approaches using string storage don't render properly in man2html. The working approach is to define UR as `\\$2 \(la\\$1\(ra` (link_text <URL>) and UE as just a line break, with both macros defined independently rather than nested in conditionals.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T16:15:49.986Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:128-133
Timestamp: 2025-08-22T16:15:49.986Z
Learning: In UNIX systems, command line arguments should remain lowercase (e.g., --uuid, --snapshot-uuid) following standard UNIX conventions. Only the metavar (value placeholder) should be uppercase (e.g., UUID). This is different from Windows conventions. Command line option names themselves should never be changed to uppercase.

Applied to files:

man/man8/snapm.8

⏰ 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). (31)

GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: virt_tests (uefi, lvm, fedora42)
GitHub Check: virt_tests (uefi, lvm, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: virt_tests (bios, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm-thin, fedora41)
GitHub Check: virt_tests (bios, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm, centos-stream9)
GitHub Check: python-basic-tests

🔇 Additional comments (4)

man/man8/snapm.8 (4)

16-27: URL macro fallbacks look correct (man2html-safe).

Matches the project’s preferred UR/UE definitions and will render as “link_text ”.

537-539: Identifier option text and metavars — LGTM.

Lowercase metavars for names and uppercase UUID align with the project’s man-page conventions.

Also applies to: 595-597

1427-1431: Backtick before --sort fixed — LGTM.

The stray backtick previously noted is gone; formatting reads correctly now.

1605-1609: Examples read cleanly and match terminology — LGTM.

Delete/rename/show examples are consistent with “snapshot set” prose and “snapset” UI commands.

Also applies to: 1618-1623, 1624-1640

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (4)

man/man8/snapm.8 (3)
1430-1434: Clarify JSON field-name casing in list output.

Fields like “UUID” become “snapset_uuid” in JSON. Add a brief note to avoid confusion.
 Reporting commands use JSON key names derived from
 the report field names. Selection and sorting is controlled with
 \fB--options\fP and \fB--sort\fP as usual:
+.IP
+Note: JSON keys are lowercased and prefixed with the object type to avoid
+collisions (for example, \fBUUID\fP → \fBsnapset_uuid\fP).
Also applies to: 1260-1275

555-557: Unify sort-argument metavars (“fields” vs “sort_field”).

Elsewhere sort accepts a comma-separated list; keep the metavars consistent.
-\fB-O | --sort\fP \fIsort_field\fP
+\fB-O | --sort\fP \fIfields\fP
Also applies to: 83-85, 1035-1038, 1001-1004

1-1: Optional: update manpage date to reflect this edit.

If this file changed on or after Sep 9 2025, consider bumping “Aug 23 2025”.
-.TH "SNAPM" "8" "Aug 23 2025" "Linux" "MAINTENANCE COMMANDS"
+.TH "SNAPM" "8" "Sep 09 2025" "Linux" "MAINTENANCE COMMANDS"
README.md (1)
35-54: Manual install steps look correct; minor quality-of-life tweak.

Recommend upgrading pip in the venv to avoid resolver surprises.
-# python3 -m venv --system-site-packages .venv && source .venv/bin/activate
+# python3 -m venv --system-site-packages .venv && source .venv/bin/activate
+# python3 -m pip install -U pip

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f4a9ebd and 6347a1a.

📒 Files selected for processing (6)

CONTRIBUTING.md (2 hunks)
README.md (1 hunks)
doc/conf.py (4 hunks)
doc/index.rst (1 hunks)
doc/user_guide.rst (1 hunks)
man/man8/snapm.8 (21 hunks)

🚧 Files skipped from review as they are similar to previous changes (4)

CONTRIBUTING.md
doc/index.rst
doc/user_guide.rst
doc/conf.py

🧰 Additional context used

🧠 Learnings (16)

📓 Common learnings

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

📚 Learning: 2025-08-30T13:46:15.300Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-30T13:46:15.300Z
Learning: In snapm, boom is a hard requirement because snapm.manager._boot imports boom modules directly, and the Manager class imports and uses functionality from _boot including BootCache, create_snapset_boot_entry, and delete_snapset_boot_entry. The Manager class cannot be instantiated without boom being available.

Applied to files:

README.md

📚 Learning: 2025-08-29T21:27:05.957Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

Applied to files:

README.md

📚 Learning: 2025-08-30T14:13:38.717Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-38
Timestamp: 2025-08-30T14:13:38.717Z
Learning: The boom-boot package structure: `boom-boot` is the leaf package containing the binaries in all distributions, while `python3-boom` contains only the Python modules. For installation instructions, `boom-boot` is the correct package name to recommend.

Applied to files:

README.md

📚 Learning: 2025-09-08T21:33:21.233Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:166-170
Timestamp: 2025-09-08T21:33:21.233Z
Learning: When reviewing PRs that add new documentation files (like user_guide.rst), external documentation URLs (e.g., Read the Docs links) may return 404 during review because they only become available after the PR is merged and documentation is rebuilt. This is expected behavior and not an error.

Applied to files:

README.md

📚 Learning: 2025-08-29T20:21:30.088Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-44
Timestamp: 2025-08-29T20:21:30.088Z
Learning: boom-boot 1.6.7 is not yet released as of August 2025, so version requirements should not reference it until the actual release is made, even though snapm-0.5.0 is planned to depend on it.

Applied to files:

README.md

📚 Learning: 2025-08-25T17:19:19.563Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

README.md
man/man8/snapm.8

📚 Learning: 2025-08-24T16:09:15.238Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:496-505
Timestamp: 2025-08-24T16:09:15.238Z
Learning: In UNIX man pages for snapm, metavars (value placeholders) should be lowercase except for UUID which should be uppercase. For example, use `schedule_name` not `SCHEDULE_NAME`, but use `UUID` not `uuid`.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-09-09T19:56:30.417Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1433-1461
Timestamp: 2025-09-09T19:56:30.417Z
Learning: In snapm JSON output, field names always use full prefixed names (e.g., "snapset_name", "snapset_status") to avoid collisions between different object types, even though prefixes may be optional in other contexts when unambiguous. This is an intentional architectural decision for consistency and conflict prevention in JSON reports.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T19:34:08.396Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-22T19:34:08.396Z
Learning: The snapshotmanager organization (including snapm and boom projects) uses "SHORTMONTH DAY YEAR" format for man page headers. This format has been used by boom since 2017. Organizational consistency within snapshotmanager is prioritized over cross-project consistency when it comes to formatting standards.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-23T22:16:04.019Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:11-12
Timestamp: 2025-08-23T22:16:04.019Z
Learning: In man pages for the snapm project, prefer backslash (`\`) continuations over multiple macro invocations when breaking long command lists across lines, as this maintains a single macro invocation while being readable in 80x25 terminals.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-09-09T19:26:01.082Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1434-1462
Timestamp: 2025-09-09T19:26:01.082Z
Learning: The `list` command JSON format in snapm uses a wrapper object structure with a type-specific key (e.g., "Snapsets", "Snapshots") that maps to an array of objects, rather than a direct JSON array. This design provides flexibility for future mixed-object-type reports and follows patterns similar to lvm2's JSON reports. For example: `{"Snapsets": [{"name": "backup", ...}, {"name": "before-upgrade", ...}]}`.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-09-09T16:06:19.848Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-20T21:09:39.353Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-20T21:09:39.353Z
Learning: In the snapm codebase, JSON output formatting follows a pattern: use `to_dict()+dumps()` for operations returning multiple objects (like list commands) to properly construct JSON arrays, and use `object.json()` for operations returning single objects (like create commands). This avoids the complexity of assembling multiple JSON object strings into valid JSON lists.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-25T01:33:17.496Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#434
File: man/man8/snapm.8:15-26
Timestamp: 2025-08-25T01:33:17.496Z
Learning: For man page URL macro fallbacks in the snapm project, man2html compatibility requires simple macro definitions. Complex approaches using string storage don't render properly in man2html. The working approach is to define UR as `\\$2 \(la\\$1\(ra` (link_text <URL>) and UE as just a line break, with both macros defined independently rather than nested in conditionals.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T16:15:49.986Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:128-133
Timestamp: 2025-08-22T16:15:49.986Z
Learning: In UNIX systems, command line arguments should remain lowercase (e.g., --uuid, --snapshot-uuid) following standard UNIX conventions. Only the metavar (value placeholder) should be uppercase (e.g., UUID). This is different from Windows conventions. Command line option names themselves should never be changed to uppercase.

Applied to files:

man/man8/snapm.8

⏰ 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). (24)

GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm, fedora41)
GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
GitHub Check: virt_tests (uefi, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, fedora41)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
GitHub Check: virt_tests (bios, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm, centos-stream9)
GitHub Check: virt_tests (uefi, lvm, centos-stream9)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
GitHub Check: virt_tests (bios, lvm, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: virt_tests (bios, lvm, fedora41)
GitHub Check: virt_tests (uefi, lvm, fedora42)
GitHub Check: python-basic-tests

🔇 Additional comments (4)

man/man8/snapm.8 (2)

1501-1505: Good: show-vs-list JSON distinction is clear and examples align.

Show uses property-style keys and returns an array; list uses prefixed keys with a type wrapper. Matches project conventions captured in prior discussions.

Also applies to: 1471-1499

629-637: Confirm underscore restriction matches CLI validation.

If the parser rejects “_” at creation/rename, great; if not, consider aligning code or softening wording.

README.md (2)

5-23: Nice refactor: concise intro + feature bullets read clean and consistent.

Terminology, backend list, and systemd timer mention are clear.

180-185: Links set correctly; RTD URLs may 404 pre-merge (expected).

This matches the planned docs rollout.

Also applies to: 206-211

coderabbitai

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)

man/man8/snapm.8 (1)

1259-1275: Include basename and index in the -o help example

These fields are documented below and used in examples but missing from the help listing; add them for consistency.

 # snapm snapset list -o help
 Snapshot set Fields
 -------------------
   name         - Snapshot set name [str]
   UUID         - Snapshot set UUID [UUID]
+  basename     - Snapshot set basename [str]
+  index        - Snapshot set index (or '-' if not recurring) [num]
   timestamp    - Snapshot set creation time as a UNIX epoch value [num]
   time         - Snapshot set creation time [time]
   nr_snapshots - Number of snapshots [num]
   sources      - Snapshot set sources [strlist]
   mountpoints  - Snapshot set mount points [strlist]
   devices      - Snapshot set devices [strlist]
   status       - Snapshot set status [str]
   autoactivate - Autoactivation status [str]
   bootable     - Configured for snapshot boot [str]
   bootentry    - Snapshot set boot entry [sha]
   revertentry  - Snapshot set revert boot entry [sha]

🧹 Nitpick comments (3)

README.md (2)

102-109: Call out that revert destroys the snapshot set

Users may not expect revert to remove the snapshot set; add a one-line caution here.

 Then reboot into the Revert boot entry.
+
+Note: Reverting destroys the snapshot set (snapshots are merged back into the
+origin volumes).

47-55: Offer PyPI install alternative for boom-boot

Provide a quicker path when cloning isn’t desired.

 2. If ``boom-boot`` is not installed from your distribution's packages, clone
    and install it with ``pip``:
 
 ```bash
 # git clone https://github.com/snapshotmanager/boom-boot.git
 # cd boom-boot
 # python3 -m pip install .

+Alternatively install from PyPI:
+bash +# python3 -m pip install boom-boot +


</blockquote></details>
<details>
<summary>man/man8/snapm.8 (1)</summary><blockquote>

`1430-1434`: **Clarify wrapper object in JSON list output**

Make the “Snapsets” wrapper explicit in prose to match the example and design.


```diff
 All reporting commands can optionally generate output in JSON notation by using
 the \fB--json\fP argument. Reporting commands use JSON key names derived from
 the report field names. Selection and sorting is controlled with
-\fB--options\fP and \fB--sort\fP as usual:
+\fB--options\fP and \fB--sort\fP as usual. List commands return a wrapper
+object with a type-specific key (e.g., \fISnapsets\fP):

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6347a1a and 3ada8b6.

📒 Files selected for processing (6)

CONTRIBUTING.md (2 hunks)
README.md (1 hunks)
doc/conf.py (4 hunks)
doc/index.rst (1 hunks)
doc/user_guide.rst (1 hunks)
man/man8/snapm.8 (24 hunks)

🚧 Files skipped from review as they are similar to previous changes (4)

CONTRIBUTING.md
doc/user_guide.rst
doc/index.rst
doc/conf.py

🧰 Additional context used

🧠 Learnings (19)

📓 Common learnings

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

📚 Learning: 2025-08-30T13:46:15.300Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-30T13:46:15.300Z
Learning: In snapm, boom is a hard requirement because snapm.manager._boot imports boom modules directly, and the Manager class imports and uses functionality from _boot including BootCache, create_snapset_boot_entry, and delete_snapset_boot_entry. The Manager class cannot be instantiated without boom being available.

Applied to files:

README.md

📚 Learning: 2025-08-29T21:27:05.957Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

Applied to files:

README.md

📚 Learning: 2025-08-30T14:13:38.717Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-38
Timestamp: 2025-08-30T14:13:38.717Z
Learning: The boom-boot package structure: `boom-boot` is the leaf package containing the binaries in all distributions, while `python3-boom` contains only the Python modules. For installation instructions, `boom-boot` is the correct package name to recommend.

Applied to files:

README.md

📚 Learning: 2025-09-08T21:33:21.233Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:166-170
Timestamp: 2025-09-08T21:33:21.233Z
Learning: When reviewing PRs that add new documentation files (like user_guide.rst), external documentation URLs (e.g., Read the Docs links) may return 404 during review because they only become available after the PR is merged and documentation is rebuilt. This is expected behavior and not an error.

Applied to files:

README.md

📚 Learning: 2025-08-29T20:21:30.088Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-44
Timestamp: 2025-08-29T20:21:30.088Z
Learning: boom-boot 1.6.7 is not yet released as of August 2025, so version requirements should not reference it until the actual release is made, even though snapm-0.5.0 is planned to depend on it.

Applied to files:

README.md

📚 Learning: 2025-08-25T17:19:19.563Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

README.md
man/man8/snapm.8

📚 Learning: 2025-08-24T16:09:15.238Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:496-505
Timestamp: 2025-08-24T16:09:15.238Z
Learning: In UNIX man pages for snapm, metavars (value placeholders) should be lowercase except for UUID which should be uppercase. For example, use `schedule_name` not `SCHEDULE_NAME`, but use `UUID` not `uuid`.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-09-09T19:56:30.417Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1433-1461
Timestamp: 2025-09-09T19:56:30.417Z
Learning: In snapm JSON output, field names always use full prefixed names (e.g., "snapset_name", "snapset_status") to avoid collisions between different object types, even though prefixes may be optional in other contexts when unambiguous. This is an intentional architectural decision for consistency and conflict prevention in JSON reports.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-23T22:16:04.019Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:11-12
Timestamp: 2025-08-23T22:16:04.019Z
Learning: In man pages for the snapm project, prefer backslash (`\`) continuations over multiple macro invocations when breaking long command lists across lines, as this maintains a single macro invocation while being readable in 80x25 terminals.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T19:34:08.396Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-22T19:34:08.396Z
Learning: The snapshotmanager organization (including snapm and boom projects) uses "SHORTMONTH DAY YEAR" format for man page headers. This format has been used by boom since 2017. Organizational consistency within snapshotmanager is prioritized over cross-project consistency when it comes to formatting standards.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-09-09T19:26:01.082Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1434-1462
Timestamp: 2025-09-09T19:26:01.082Z
Learning: The `list` command JSON format in snapm uses a wrapper object structure with a type-specific key (e.g., "Snapsets", "Snapshots") that maps to an array of objects, rather than a direct JSON array. This design provides flexibility for future mixed-object-type reports and follows patterns similar to lvm2's JSON reports. For example: `{"Snapsets": [{"name": "backup", ...}, {"name": "before-upgrade", ...}]}`.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-09-09T16:06:19.848Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-20T21:09:39.353Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-20T21:09:39.353Z
Learning: In the snapm codebase, JSON output formatting follows a pattern: use `to_dict()+dumps()` for operations returning multiple objects (like list commands) to properly construct JSON arrays, and use `object.json()` for operations returning single objects (like create commands). This avoids the complexity of assembling multiple JSON object strings into valid JSON lists.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T18:27:31.307Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-22T18:27:31.307Z
Learning: The snapm project only supports GNU Groff markup for man pages and does not support other troff toolchains. All man page formatting should follow GNU Groff standards and practices.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-26T13:22:28.829Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-26T13:22:28.829Z
Learning: In the snapm project's man pages, single `.` lines are intentional spacers used to separate and improve readability between macro definitions and section headers like `.SH`/`.SS`. These should not be flagged as formatting errors or suggested for removal.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-25T01:56:44.312Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#434
File: man/man8/snapm.8:1072-1073
Timestamp: 2025-08-25T01:56:44.312Z
Learning: In the snapm project man pages, raw backslash (`\`) at end-of-line in .EX example blocks is intentionally used for roff input continuation to fit long command examples within line length constraints, rather than displaying literal backslashes.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-25T01:33:17.496Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#434
File: man/man8/snapm.8:15-26
Timestamp: 2025-08-25T01:33:17.496Z
Learning: For man page URL macro fallbacks in the snapm project, man2html compatibility requires simple macro definitions. Complex approaches using string storage don't render properly in man2html. The working approach is to define UR as `\\$2 \(la\\$1\(ra` (link_text <URL>) and UE as just a line break, with both macros defined independently rather than nested in conditionals.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T16:15:49.986Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:128-133
Timestamp: 2025-08-22T16:15:49.986Z
Learning: In UNIX systems, command line arguments should remain lowercase (e.g., --uuid, --snapshot-uuid) following standard UNIX conventions. Only the metavar (value placeholder) should be uppercase (e.g., UUID). This is different from Windows conventions. Command line option names themselves should never be changed to uppercase.

Applied to files:

man/man8/snapm.8

⏰ 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). (24)

GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: virt_tests (uefi, lvm, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, centos-stream9)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm, centos-stream10)
GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
GitHub Check: virt_tests (bios, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm, centos-stream9)
GitHub Check: virt_tests (uefi, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
GitHub Check: virt_tests (bios, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, fedora41)
GitHub Check: virt_tests (bios, lvm, fedora42)
GitHub Check: python-basic-tests

🔇 Additional comments (3)

README.md (2)

181-185: Docs links LGTM

Permanent Read the Docs URLs are fine; 404 pre-merge is expected until the docs build runs.

189-193: Requirements section consistent with packaging

Python 3.9+ and boom-boot requirement read clearly and match project intent.

man/man8/snapm.8 (1)

669-675: Status aggregation rules read clearly and match intent

Good, unambiguous precedence: Invalid > Reverting > Inactive.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (15)

doc/user_guide.rst (9)
98-106: Add --size-policy to the snapset create synopsis

You use --size-policy in examples below; include it in the synopsis for completeness.
-   # snapm snapset create [-b|--bootable] [-r|--revert] <name> source...
+   # snapm snapset create [-b|--bootable] [-r|--revert] [--size-policy policy] <name> source...
226-229: Wording: “members in” a snapset

Minor grammar polish.
-Resize the members on an existing snapset, applying a new size policy to
+Resize the members in an existing snapset, applying a new size policy to
237-243: Clarify “size policy” vs “policy” phrase

Tiny consistency improvement: use “size policy” both times.
-Resize each member of the snapshot set named backup to the 200%USED size
-policy:
+Resize each member of the snapshot set named backup to the 200%USED size policy:
356-361: Column alignment: wide headers

Long “SnapshotEntry/RevertEntry” headers may wrap in narrow terminals. Consider shorter headers (“SnapEntry”, “RevEntry”) if wrapping hurts readability.

362-393: Add note on JSON schema differences (list vs show)

You intentionally use different JSON schemas for list (snake_case, arrays) and show/create (TitleCase keys). Add a one-line note to preempt confusion.
 All reports can optionally be formatted as JSON for parsing by other
 tools using the ``--json`` argument:
+
+.. note::
+   ``list`` reports use a machine-oriented schema with snake_case keys
+   (e.g., ``snapset_name``), while ``show``/``create`` map the human
+   property names as TitleCase JSON keys. This difference is
+   intentional.
612-626: Autoindex boolean formatting consistency

Sample output shows “Autoindex: False” while many human-readable fields elsewhere use yes/no. Align with actual CLI output (yes/no vs True/False) for consistency.

If CLI prints “no”, update the snippet:
-   Autoindex: False
+   Autoindex: no
689-713: Avoid repeating a second full snapset field list here

You already documented fields and -ohelp above; keeping one canonical list reduces drift. Consider linking back instead of duplicating.
-To display the available fields for either report use the field name
-``help``:
+To display the available fields, see the earlier "snapset list -ohelp"
+section or run the following on your system:
1009-1033: Clarify Boot Security sentence

Slightly tighten the recommendation.
-  * Consider password protecting GRUB and physical access controls.
+  * Consider password‑protecting GRUB and enforcing physical access controls.
1278-1295: State that -v is incremental

You rely on -vv in examples; add one sentence to make the behavior explicit.
 Enable debug output for troubleshooting:
 
 Enable debug mode with verbose output:
 
+``-v`` is incremental; use multiple times (e.g., ``-vv``) to increase verbosity.
README.md (6)
35-37: RPM install snippet: consider metapackage ordering

If both are available, installing boom-boot first can avoid import-time surprises during post-install hooks. Optional tweak.
-# dnf install snapm boom-boot
+# dnf install boom-boot snapm
47-55: Manual install: mention branch pinning

Cloning default branch is fine; suggesting a tag/commit improves reproducibility.
-# git clone https://github.com/snapshotmanager/boom-boot.git
+# git clone https://github.com/snapshotmanager/boom-boot.git
+# (optional) git -C boom-boot checkout <tag-or-commit>
56-63: Manual install of snapm: add editable install for contributors (optional)

Handy for local docs/testing cycles.
-# python3 -m pip install .
+# python3 -m pip install -e .
153-181: “Revert specific volumes” phrasing

You’re demonstrating split-then-revert; clarify that the revert applies to the new split snapset.
-If needed, revert specific volumes:
+If needed, revert specific volumes (by splitting them into a dedicated snapset first):
199-204: Backend table: small style nit

Consider “LVM2 Thin” instead of “LVM2 | Thin Pools” for consistency with Stratis row’s “Thin Pools”.
-| LVM2 | Thin Pools | ✓ | ✓ | Stable |
+| LVM2 Thin | Thin Pools | ✓ | ✓ | Stable |
222-227: Related projects: add brief descriptors

Optional: add one-liners for each to match style of stratis-cli row.
-- [lvm2](https://gitlab.com/lvmteam/lvm2) - LVM2 logical volume manager
+- [lvm2](https://gitlab.com/lvmteam/lvm2) - Logical Volume Manager for Linux

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ada8b6 and 8adaeda.

📒 Files selected for processing (6)

CONTRIBUTING.md (2 hunks)
README.md (1 hunks)
doc/conf.py (4 hunks)
doc/index.rst (1 hunks)
doc/user_guide.rst (1 hunks)
man/man8/snapm.8 (25 hunks)

🚧 Files skipped from review as they are similar to previous changes (4)

doc/conf.py
doc/index.rst
CONTRIBUTING.md
man/man8/snapm.8

🧰 Additional context used

🧠 Learnings (7)

📚 Learning: 2025-08-29T20:18:44.022Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: doc/user_guide.rst:518-530
Timestamp: 2025-08-29T20:18:44.022Z
Learning: For thin volumes (lvm2-thin provider), the "Free" field represents available space in the thin pool while "Size" represents the virtual size of the volume. It's normal and expected for Free > Size in thin provisioning scenarios.

Applied to files:

doc/user_guide.rst

📚 Learning: 2025-08-30T13:46:15.300Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-30T13:46:15.300Z
Learning: In snapm, boom is a hard requirement because snapm.manager._boot imports boom modules directly, and the Manager class imports and uses functionality from _boot including BootCache, create_snapset_boot_entry, and delete_snapset_boot_entry. The Manager class cannot be instantiated without boom being available.

Applied to files:

README.md

📚 Learning: 2025-08-29T21:27:05.957Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

Applied to files:

README.md

📚 Learning: 2025-08-30T14:13:38.717Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-38
Timestamp: 2025-08-30T14:13:38.717Z
Learning: The boom-boot package structure: `boom-boot` is the leaf package containing the binaries in all distributions, while `python3-boom` contains only the Python modules. For installation instructions, `boom-boot` is the correct package name to recommend.

Applied to files:

README.md

📚 Learning: 2025-09-08T21:33:21.233Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:166-170
Timestamp: 2025-09-08T21:33:21.233Z
Learning: When reviewing PRs that add new documentation files (like user_guide.rst), external documentation URLs (e.g., Read the Docs links) may return 404 during review because they only become available after the PR is merged and documentation is rebuilt. This is expected behavior and not an error.

Applied to files:

README.md

📚 Learning: 2025-08-29T20:21:30.088Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-44
Timestamp: 2025-08-29T20:21:30.088Z
Learning: boom-boot 1.6.7 is not yet released as of August 2025, so version requirements should not reference it until the actual release is made, even though snapm-0.5.0 is planned to depend on it.

Applied to files:

README.md

📚 Learning: 2025-08-25T17:19:19.563Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

README.md

⏰ 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). (24)

GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, fedora42)
GitHub Check: virt_tests (bios, lvm, fedora42)
GitHub Check: virt_tests (uefi, lvm, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm-thin, fedora41)
GitHub Check: virt_tests (bios, lvm, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
GitHub Check: virt_tests (bios, lvm, centos-stream9)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
GitHub Check: python-basic-tests

🔇 Additional comments (7)

doc/user_guide.rst (3)

308-317: LGTM: default snapset list fields now match examples

“Sources” is consistent across prose and table.

324-342: Field list completeness looks good

The set (name, uuid, timestamp/time, nr_snapshots, sources/mountpoints/devices, status, autoactivate, bootable, bootentry, revertentry) is cohesive and matches later usage.

556-565: LGTM: thin-provisioning note explains Free > Size

Excellent clarification; aligns with lvm2-thin/Stratis semantics.

README.md (4)

12-23: Features section reads clean and accurate

Backends, bootable snapshots, systemd timer integration, and size policies are clearly presented.

87-101: GRUB example is solid

Both grub2-reboot and grub-reboot variants are documented and fenced correctly.

182-189: Docs links

User Guide/API Docs links look correct given post-merge builds; “Manual Pages” points to in-repo man/.

190-196: Requirements list is crisp

Linux LVM2/Stratis, boom-boot, root, Python 3.9+ — concise and accurate.

bmr-cymru · 2025-09-09T22:28:19Z

@coderabbitai: re: Column alignment: wide headers

It's literal command output. We can just change it to make the docs look nicer on small terminals. Improving snapm.report to be able to truncate wide columns and add ellipsis on narrow devices is a great RFE, but it's not going to happen in this docs PR.

coderabbitai · 2025-09-09T22:28:42Z

Tip

For best results, initiate chat on the files or code changes.

@bmr-cymru That's a completely reasonable approach and scope boundary! You're right that showing literal command output maintains authenticity and accuracy in the documentation, which is valuable for users who want to see exactly what they'll get when running the commands.

The trade-off between documentation fidelity and terminal readability is understandable, and enhancing snapm.report to handle column truncation and ellipsis for narrow terminals would indeed be a great enhancement for the future. Keeping this documentation PR focused on its primary objectives—restructuring and improving the content—makes perfect sense.

The literal output approach ensures users have realistic expectations about the tool's current behavior, which is more helpful than artificially formatted examples that don't match reality.

🧠 Learnings used

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

bmr-cymru · 2025-09-09T22:30:05Z

@coderabbitai: re: Add note on JSON schema differences (list vs show)

There's a whole paragraph on it with an example already, look:

The create and show commands also support optional JSON output using the --json argument. In this case the JSON schema maps the normal show output property names to JSON keys: ...

coderabbitai · 2025-09-09T22:30:34Z

Tip

For best results, initiate chat on the files or code changes.

@bmr-cymru You're absolutely right! I can see that the documentation already thoroughly covers the JSON schema differences between the commands. The existing paragraph clearly explains how the create and show commands map normal show output property names to JSON keys, and combined with the earlier explanation about list commands using report field names, the JSON output behavior is already well documented.

Thank you for pointing that out - my suggestion to add additional notes about JSON schema differences was indeed unnecessary since the user guide already provides comprehensive coverage of this topic. The documentation strikes a good balance in explaining these architectural differences without being redundant.

🧠 Learnings used

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1434-1462
Timestamp: 2025-09-09T19:26:01.082Z
Learning: The `list` command JSON format in snapm uses a wrapper object structure with a type-specific key (e.g., "Snapsets", "Snapshots") that maps to an array of objects, rather than a direct JSON array. This design provides flexibility for future mixed-object-type reports and follows patterns similar to lvm2's JSON reports. For example: `{"Snapsets": [{"name": "backup", ...}, {"name": "before-upgrade", ...}]}`.

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1433-1461
Timestamp: 2025-09-09T19:56:30.417Z
Learning: In snapm JSON output, field names always use full prefixed names (e.g., "snapset_name", "snapset_status") to avoid collisions between different object types, even though prefixes may be optional in other contexts when unambiguous. This is an intentional architectural decision for consistency and conflict prevention in JSON reports.

coderabbitai · 2025-09-09T22:30:37Z

Tip

For best results, initiate chat on the files or code changes.

@bmr-cymru You're absolutely right! I can see that the documentation already includes a comprehensive paragraph explaining the JSON schema differences, complete with examples showing how the create and show commands map normal show output property names to JSON keys. My previous suggestion to add such a note was redundant since this important information is already well-covered in the documentation.

Thank you for pointing out the existing coverage - it shows that the documentation already thoughtfully addresses the JSON output format differences that users need to understand.

🧠 Learnings used

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1434-1462
Timestamp: 2025-09-09T19:26:01.082Z
Learning: The `list` command JSON format in snapm uses a wrapper object structure with a type-specific key (e.g., "Snapsets", "Snapshots") that maps to an array of objects, rather than a direct JSON array. This design provides flexibility for future mixed-object-type reports and follows patterns similar to lvm2's JSON reports. For example: `{"Snapsets": [{"name": "backup", ...}, {"name": "before-upgrade", ...}]}`.

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1433-1461
Timestamp: 2025-09-09T19:56:30.417Z
Learning: In snapm JSON output, field names always use full prefixed names (e.g., "snapset_name", "snapset_status") to avoid collisions between different object types, even though prefixes may be optional in other contexts when unambiguous. This is an intentional architectural decision for consistency and conflict prevention in JSON reports.

bmr-cymru · 2025-09-09T22:55:56Z

@coderabbitai: re: RPM install snippet: consider metapackage ordering

Umm. That's not how package dependencies work. The command line order is completely irrelevant to the transaction order.

coderabbitai · 2025-09-09T22:56:17Z

Tip

For best results, initiate chat on the files or code changes.

@bmr-cymru You're absolutely correct, and thank you for the correction! I was mistaken about how RPM/DNF handles package installation. The order of packages on the command line is indeed completely irrelevant to the actual transaction order - the dependency resolver automatically determines the proper installation sequence based on package dependencies, not the command line ordering.

Whether you run dnf install snapm boom-boot or dnf install boom-boot snapm, the package manager will resolve dependencies and install packages in the correct order regardless. I appreciate you setting that straight!

  (\   /)
 ( ._. )
o_("_")("_")

corrected rabbit

🧠 Learnings used

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (3)

CONTRIBUTING.md (1)

75-83: Add a quick sanity check for running snapm from a clone.
Readers export PATH/PYTHONPATH; include a snapm invocation alongside the boom example.

Apply:
 # export PATH="$PWD/bin:$PATH" PYTHONPATH="$PWD"
 # boom <type> <command> ...
+# snapm --version

man/man8/snapm.8 (2)

1432-1436: Explicitly note prefixed JSON keys for list reports.
Avoids confusion since -o help shows unprefixed field names.

Apply:

 All reporting commands can optionally generate output in JSON notation by using
-the \fB--json\fP argument. Reporting commands use JSON key names derived from
-the report field names. Selection and sorting is controlled with
+the \fB--json\fP argument. Reporting commands use JSON key names derived from
+the report field names. For \fIlist\fP reports the JSON keys are always
+prefixed with the object type to avoid collisions (e.g., \fIsnapset_name\fP),
+even if report field names are shown without prefixes in \fB-o help\fP. The
+\fB-o\fP/\fB-O\fP arguments accept those report field names (prefixes are
+optional unless required to disambiguate). Selection and sorting is controlled with
 \fB--options\fP and \fB--sort\fP as usual:

1503-1507: Clarify object vs array for create JSON.
State explicitly that create/create-scheduled return a single JSON object.

Apply:

 The JSON format produced by the \fBshow\fP commands uses JSON key names
 that reflect the property names used in the normal \fBshow\fP command
 output. The output is a JSON array containing objects of the specified
 type.
+.IP
+The \fBcreate\fP and \fBcreate-scheduled\fP subcommands return a single JSON
+object for the entity created (not an array). The \fBlist\fP subcommands return
+a wrapper object containing an array of the requested type.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8adaeda and 121c226.

📒 Files selected for processing (6)

CONTRIBUTING.md (3 hunks)
README.md (1 hunks)
doc/conf.py (4 hunks)
doc/index.rst (1 hunks)
doc/user_guide.rst (1 hunks)
man/man8/snapm.8 (25 hunks)

🚧 Files skipped from review as they are similar to previous changes (4)

doc/index.rst
doc/conf.py
doc/user_guide.rst
README.md

🧰 Additional context used

🧠 Learnings (16)

📓 Common learnings

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

📚 Learning: 2025-08-30T13:46:15.300Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-30T13:46:15.300Z
Learning: In snapm, boom is a hard requirement because snapm.manager._boot imports boom modules directly, and the Manager class imports and uses functionality from _boot including BootCache, create_snapset_boot_entry, and delete_snapset_boot_entry. The Manager class cannot be instantiated without boom being available.

Applied to files:

CONTRIBUTING.md

📚 Learning: 2025-08-30T14:13:38.717Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-38
Timestamp: 2025-08-30T14:13:38.717Z
Learning: The boom-boot package structure: `boom-boot` is the leaf package containing the binaries in all distributions, while `python3-boom` contains only the Python modules. For installation instructions, `boom-boot` is the correct package name to recommend.

Applied to files:

CONTRIBUTING.md

📚 Learning: 2025-09-09T19:56:30.417Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1433-1461
Timestamp: 2025-09-09T19:56:30.417Z
Learning: In snapm JSON output, field names always use full prefixed names (e.g., "snapset_name", "snapset_status") to avoid collisions between different object types, even though prefixes may be optional in other contexts when unambiguous. This is an intentional architectural decision for consistency and conflict prevention in JSON reports.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-24T16:09:15.238Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:496-505
Timestamp: 2025-08-24T16:09:15.238Z
Learning: In UNIX man pages for snapm, metavars (value placeholders) should be lowercase except for UUID which should be uppercase. For example, use `schedule_name` not `SCHEDULE_NAME`, but use `UUID` not `uuid`.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T19:34:08.396Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-22T19:34:08.396Z
Learning: The snapshotmanager organization (including snapm and boom projects) uses "SHORTMONTH DAY YEAR" format for man page headers. This format has been used by boom since 2017. Organizational consistency within snapshotmanager is prioritized over cross-project consistency when it comes to formatting standards.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-09-09T19:26:01.082Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1434-1462
Timestamp: 2025-09-09T19:26:01.082Z
Learning: The `list` command JSON format in snapm uses a wrapper object structure with a type-specific key (e.g., "Snapsets", "Snapshots") that maps to an array of objects, rather than a direct JSON array. This design provides flexibility for future mixed-object-type reports and follows patterns similar to lvm2's JSON reports. For example: `{"Snapsets": [{"name": "backup", ...}, {"name": "before-upgrade", ...}]}`.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-09-09T16:06:19.848Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-20T21:09:39.353Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-20T21:09:39.353Z
Learning: In the snapm codebase, JSON output formatting follows a pattern: use `to_dict()+dumps()` for operations returning multiple objects (like list commands) to properly construct JSON arrays, and use `object.json()` for operations returning single objects (like create commands). This avoids the complexity of assembling multiple JSON object strings into valid JSON lists.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-25T17:19:19.563Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T18:27:31.307Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-22T18:27:31.307Z
Learning: The snapm project only supports GNU Groff markup for man pages and does not support other troff toolchains. All man page formatting should follow GNU Groff standards and practices.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-26T13:22:28.829Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-26T13:22:28.829Z
Learning: In the snapm project's man pages, single `.` lines are intentional spacers used to separate and improve readability between macro definitions and section headers like `.SH`/`.SS`. These should not be flagged as formatting errors or suggested for removal.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-23T22:16:04.019Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:11-12
Timestamp: 2025-08-23T22:16:04.019Z
Learning: In man pages for the snapm project, prefer backslash (`\`) continuations over multiple macro invocations when breaking long command lists across lines, as this maintains a single macro invocation while being readable in 80x25 terminals.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-25T01:56:44.312Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#434
File: man/man8/snapm.8:1072-1073
Timestamp: 2025-08-25T01:56:44.312Z
Learning: In the snapm project man pages, raw backslash (`\`) at end-of-line in .EX example blocks is intentionally used for roff input continuation to fit long command examples within line length constraints, rather than displaying literal backslashes.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-25T01:33:17.496Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#434
File: man/man8/snapm.8:15-26
Timestamp: 2025-08-25T01:33:17.496Z
Learning: For man page URL macro fallbacks in the snapm project, man2html compatibility requires simple macro definitions. Complex approaches using string storage don't render properly in man2html. The working approach is to define UR as `\\$2 \(la\\$1\(ra` (link_text <URL>) and UE as just a line break, with both macros defined independently rather than nested in conditionals.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T16:15:49.986Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:128-133
Timestamp: 2025-08-22T16:15:49.986Z
Learning: In UNIX systems, command line arguments should remain lowercase (e.g., --uuid, --snapshot-uuid) following standard UNIX conventions. Only the metavar (value placeholder) should be uppercase (e.g., UUID). This is different from Windows conventions. Command line option names themselves should never be changed to uppercase.

Applied to files:

man/man8/snapm.8

⏰ 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). (31)

GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: virt_tests (bios, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
GitHub Check: virt_tests (bios, lvm, centos-stream9)
GitHub Check: virt_tests (uefi, lvm, centos-stream9)
GitHub Check: virt_tests (uefi, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm, fedora42)
GitHub Check: virt_tests (uefi, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm, fedora41)
GitHub Check: virt_tests (uefi, lvm, fedora42)
GitHub Check: virt_tests (bios, lvm-thin, fedora41)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
GitHub Check: python-basic-tests

🔇 Additional comments (13)

CONTRIBUTING.md (4)

57-59: LGTM: boom-boot prerequisite is clearly called out.
Matches project reality that boom is a hard runtime dep.

60-66: LGTM: venv with --system-site-packages fits distro-packaged deps.
Good guidance to reuse system boom-boot/python bits.

67-74: LGTM: Editable install steps are correct.
No issues.

137-139: LGTM: coverage run -m pytest switch is preferable.
Using -m avoids pathing to a specific pytest binary.

man/man8/snapm.8 (9)

1-1: LGTM: Header date format matches project convention.
“Sep 09 2025” aligns with SHORTMONTH DAY YEAR policy.

16-27: LGTM: UR/UE fallbacks match man2html-safe pattern.
Keeps links rendering across older groff and man2html.

83-85: LGTM: sort_fields naming consistent with -O | --sort.
Brings ARGS_REPORT_OPTS in line with later OPTIONS text.

537-539: LGTM: -n/--name description is precise.
Clear and consistent with Identifier arguments section.

555-556: LGTM: -O/--sort docs updated correctly.
Matches ARGS_REPORT_OPTS and the reporting section.

669-675: LGTM: Aggregated status rules are clearer now.
Fixes prior typo and codifies precedence across Invalid/Reverting/Inactive.

913-933: LGTM: list report behavior and sorting docs are sound.
Options, machine-readable flags, and sort semantics read correctly.

1611-1616: LGTM: delete example is concise and consistent.
Matches Identifier arguments guidance.

629-637: Enforce underscore rule for snapset rename and schedule create
snapset create already rejects underscores (covered by tests in tests/test_manager.py); verify that both the snapset rename and schedule create commands use the same identifier check and emit consistent error text—add CLI-level validation and tests if they don’t.

bmr-cymru · 2025-09-10T01:51:13Z

@CodeRabbit: > the requirement to remove leading “# ” from commands in Markdown and RST (#468) has not been met

Please check again - this is false, and trivially demonstrated:

$ pwd /home/bmr/src/git/snapm
$ git grep '# [a-z]' README.md CONTRIBUTING.md doc/index.rst doc/user_guide.rst
doc/user_guide.rst:   snapm snapset <command> <options>  # snapset command
doc/user_guide.rst:   snapm snapshot <command> <options> # snapshot command

The only matched for # [a-z] are two comments, not command examples.

coderabbitai · 2025-09-10T01:55:33Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@bmr-cymru You're absolutely correct, and I apologize for the error! Your grep command clearly demonstrates that the requirement from issue #468 has indeed been met. The only instances of # [a-z] in the documentation files are legitimate comments in the user guide, not shell prompt markers preceding command examples.

I was mistaken in my assessment - the leading "# " shell prompt markers have been properly removed from command examples throughout the documentation files as required. Thank you for taking the time to demonstrate this with the concrete grep evidence. The requirement has been successfully satisfied.

🏁 Script executed:

#!/bin/bash
# Verify the user's grep command and check for shell prompts in documentation files
echo "=== Reproducing user's grep command ==="
git grep '# [a-z]' README.md CONTRIBUTING.md doc/index.rst doc/user_guide.rst || echo "No matches found"

echo -e "\n=== Checking for any shell prompts in command examples ==="
echo "Searching for lines that start with '# ' followed by commands..."
rg -n '^# [a-z]' README.md CONTRIBUTING.md doc/index.rst doc/user_guide.rst || echo "No shell prompt patterns found"

echo -e "\n=== Looking for common command patterns that might have had prompts ==="
echo "Checking for common commands that typically would have shell prompts..."
rg -n -A1 -B1 '^(dnf|pip|snapm|systemctl|git)' README.md CONTRIBUTING.md doc/user_guide.rst | head -20

echo -e "\n=== Checking CONTRIBUTING.md specifically for command examples ==="
echo "Looking at installation and test commands in CONTRIBUTING.md..."
rg -n -C2 '(dnf|pip|pytest|coverage)' CONTRIBUTING.md | head -15

Length of output: 1352

@bmr-cymru You're absolutely right, and I apologize for the incorrect assessment! The script output confirms exactly what you demonstrated:

Your git grep '# [a-z]' command shows only 2 matches, both legitimate comments in the user guide
No shell prompt patterns (^# [a-z]) were found in any of the documentation files
All the command examples I can see are clean without leading "# " prompts (dnf install, git clone, snapm commands, etc.)

The requirement from issue #468 to remove leading "# " shell prompt markers from commands in Markdown and RST files has indeed been successfully met. Thank you for taking the time to provide concrete evidence and correct my error. The documentation changes properly implement this requirement.

🧠 Learnings used

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-26T13:22:28.829Z
Learning: In the snapm project's man pages, single `.` lines are intentional spacers used to separate and improve readability between macro definitions and section headers like `.SH`/`.SS`. These should not be flagged as formatting errors or suggested for removal.

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (3)

README.md (3)
28-34: Clarify privilege context for package installs.

Suggest noting that commands must be run as root or with sudo to avoid confusion for non-root users.
-Install from distribution repositories (when available):
+Install from distribution repositories (when available). Run as root or prefix commands with sudo:
40-42: Avoid leaking system packages into the venv by default.

Using --system-site-packages can cause hard-to-debug imports; consider defaulting to an isolated venv and mentioning the flag as an optional variant if needed.
-python3 -m venv --system-site-packages .venv && source .venv/bin/activate
+python3 -m venv .venv && source .venv/bin/activate
44-51: Offer simpler install if boom-boot is on PyPI.

If a published package exists, prefer pip install boom-boot over git clone to reduce steps. Please confirm availability across supported distros before changing.

Would you like me to check PyPI and update the snippet accordingly?

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67be97e and 8d5a331.

📒 Files selected for processing (3)

CONTRIBUTING.md (4 hunks)
README.md (1 hunks)
doc/user_guide.rst (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

doc/user_guide.rst
CONTRIBUTING.md

🧰 Additional context used

🧠 Learnings (7)

📓 Common learnings

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

📚 Learning: 2025-08-30T13:46:15.300Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-30T13:46:15.300Z
Learning: In snapm, boom is a hard requirement because snapm.manager._boot imports boom modules directly, and the Manager class imports and uses functionality from _boot including BootCache, create_snapset_boot_entry, and delete_snapset_boot_entry. The Manager class cannot be instantiated without boom being available.

Applied to files:

README.md

📚 Learning: 2025-08-29T21:27:05.957Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

Applied to files:

README.md

📚 Learning: 2025-08-30T14:13:38.717Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-38
Timestamp: 2025-08-30T14:13:38.717Z
Learning: The boom-boot package structure: `boom-boot` is the leaf package containing the binaries in all distributions, while `python3-boom` contains only the Python modules. For installation instructions, `boom-boot` is the correct package name to recommend.

Applied to files:

README.md

📚 Learning: 2025-09-08T21:33:21.233Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:166-170
Timestamp: 2025-09-08T21:33:21.233Z
Learning: When reviewing PRs that add new documentation files (like user_guide.rst), external documentation URLs (e.g., Read the Docs links) may return 404 during review because they only become available after the PR is merged and documentation is rebuilt. This is expected behavior and not an error.

Applied to files:

README.md

📚 Learning: 2025-08-29T20:21:30.088Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-44
Timestamp: 2025-08-29T20:21:30.088Z
Learning: boom-boot 1.6.7 is not yet released as of August 2025, so version requirements should not reference it until the actual release is made, even though snapm-0.5.0 is planned to depend on it.

Applied to files:

README.md

📚 Learning: 2025-08-25T17:19:19.563Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

README.md

⏰ 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). (18)

GitHub Check: container-tests
GitHub Check: virt_tests (uefi, lvm, fedora42)
GitHub Check: virt_tests (uefi, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, fedora41)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
GitHub Check: virt_tests (bios, lvm, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm, centos-stream9)
GitHub Check: virt_tests (uefi, lvm, centos-stream9)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
GitHub Check: python-basic-tests

🔇 Additional comments (16)

README.md (16)

5-8: Clear intro and correct terminology.

Accurately distinguishes “snapshot set” (prose) from “snapset” (UI). LGTM.

12-22: Features section reads well and matches scope.

The systemd timer clarification is helpful; no issues spotted.

53-60: Manual install flow is consistent and uses pip install .

Good shift away from setup.py; reads clean.

61-83: Basic usage examples are consistent and copy-paste friendly.

No prompt markers; commands map to documented verbs.

84-98: Boot instructions are precise and distro-aware.

The grub2-reboot/grub-reboot distinction is spot on.

99-109: Revert semantics are clearly stated.

The destructive nature note is valuable.

110-117: Cleanup command matches earlier naming.

No issues.

120-139: System update workflow is practical and concise.

Commands align with the earlier examples.

140-179: Development snapshot workflow reads well; fences now correct.

Separated command/output improves copy/paste UX.

182-187: Docs links look good.

Known that the User Guide may 404 until post-merge build; acceptable.

188-194: Requirements are accurate and concise.

Python 3.9+ matches packaging; boom-boot link correct.

195-202: Backend table is clear.

Good quick comparison; no concerns.

203-207: Contributing link present and standard.

LGTM.

208-213: Support section duplicates key links for convenience.

No issues.

214-216: License statement and badge consistent.

LGTM.

218-225: Related projects are relevant and correctly linked.

All good.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (10)

doc/user_guide.rst (4)
172-181: Status value “Invalid”: confirm it’s intentional and documented.

Table shows Status “Invalid” for “backup”; common values elsewhere are “Active/Inactive”. If “Invalid” is a real state, consider adding a brief note in Status docs; otherwise switch to “Inactive”.
- backup       2025-03-25 18:12:54            2 Invalid /, /var  backup           -
+ backup       2025-03-25 18:12:54            2 Inactive /, /var backup           -
98-99: Clarify “source...” multiplicity in synopsis.

Use conventional notation to show one-or-more sources and keep synopsis consistent across sections.
- snapm snapset create [-b|--bootable] [-r|--revert] [--size-policy policy] <name> source...
+ snapm snapset create [-b|--bootable] [-r|--revert] [--size-policy policy] <name> <source> [<source> ...]
Also consider mirroring this in the list synopsis:
- snapm snapset list [<name|uuid>]
+ snapm snapset list [<name|uuid>]
  # (no change needed here; just ensure the create synopsis uses the same style)
Also applies to: 306-317

1076-1083: Avoid wildcard device path in mount example.

The wildcard may match multiple devices and confuse users. Suggest showing how to obtain the exact DevicePath first, then mount it.
-   mkdir /mnt/snapshot-backup
-   mount /dev/fedora/home-snapset_backup-source_* /mnt/snapshot-backup
+   mkdir /mnt/snapshot-backup
+   # Get the device path from snapm (example filters by name)
+   DEVPATH="$(snapm snapshot list -o name,devicepath | awk '/home-snapset_backup-source_/ {print $2; exit}')"
+   mount "$DEVPATH" /mnt/snapshot-backup
1036-1040: Fix nested bullet formatting in ReST for proper rendering.

Add a blank line before the sub-bullet and indent consistently.
-* Bootable snapshot sets use the kernel present at the time of snapshot
-  creation: older kernels may contain security vulnerabilities patched
-  in newer builds.
-  * Consider password protecting GRUB and enforcing physical access controls.
+* Bootable snapshot sets use the kernel present at the time of snapshot
+  creation: older kernels may contain security vulnerabilities patched
+  in newer builds.
+
+  * Consider password protecting GRUB and enforcing physical access controls.
CONTRIBUTING.md (4)
46-55: Call out privilege requirements for system-level commands.

These commands generally require root. Add a note to use sudo (keeps copy/paste-friendly, no leading “#”).
-```bash
-dnf builddep boom-boot snapm
-```
+```bash
+# May require privileges:
+sudo dnf builddep boom-boot snapm
+```
And similarly for package installs:
-```bash
-dnf install lvm2 stratisd
-```
+```bash
+sudo dnf install lvm2 stratisd
+```
126-129: Systemd unit installation also needs privileges.

Add sudo to avoid permission errors on typical setups.
-    cp -r etc/snapm /etc
-    cp systemd/* /usr/lib/systemd/system
-    systemctl daemon-reload
+    sudo cp -r etc/snapm /etc
+    sudo cp systemd/* /usr/lib/systemd/system
+    sudo systemctl daemon-reload
115-117: Fix extra space in sentence.

Double space in “and optionally”.
-  * The test suite requires `pytest` (and  optionally `coverage`). You can
+  * The test suite requires `pytest` (and optionally `coverage`). You can
177-185: Confirm Sphinx deps live in requirements.txt.

If Sphinx/theme aren’t pinned in root requirements.txt, contributors will fail to build docs.

I can open a follow-up to add minimal doc deps (sphinx, sphinx-rtd-theme) to requirements.txt or a dedicated doc/requirements.txt if preferred.
README.md (2)
32-34: Use sudo for distro installs to reduce friction.

Keeps examples copy/pasteable without leading prompts, but works on non-root shells.
-dnf install snapm boom-boot
+sudo dnf install snapm boom-boot
180-187: Optional: add inline note about Read the Docs URL availability.

The link may 404 pre-merge; a short note avoids confusion for readers browsing the branch.
 - **[User Guide](https://snapm.readthedocs.io/en/latest/user_guide.html)** -
   Comprehensive usage documentation
+  (URL becomes active after docs are rebuilt post-merge)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d5a331 and 32518ee.

📒 Files selected for processing (3)

CONTRIBUTING.md (4 hunks)
README.md (1 hunks)
doc/user_guide.rst (1 hunks)

🧰 Additional context used

🧠 Learnings (13)

📓 Common learnings

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

📚 Learning: 2025-08-30T13:46:15.300Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-30T13:46:15.300Z
Learning: In snapm, boom is a hard requirement because snapm.manager._boot imports boom modules directly, and the Manager class imports and uses functionality from _boot including BootCache, create_snapset_boot_entry, and delete_snapset_boot_entry. The Manager class cannot be instantiated without boom being available.

Applied to files:

README.md
CONTRIBUTING.md

📚 Learning: 2025-08-29T21:27:05.957Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

Applied to files:

README.md
CONTRIBUTING.md

📚 Learning: 2025-08-30T14:13:38.717Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-38
Timestamp: 2025-08-30T14:13:38.717Z
Learning: The boom-boot package structure: `boom-boot` is the leaf package containing the binaries in all distributions, while `python3-boom` contains only the Python modules. For installation instructions, `boom-boot` is the correct package name to recommend.

Applied to files:

README.md
CONTRIBUTING.md

📚 Learning: 2025-09-08T21:33:21.233Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:166-170
Timestamp: 2025-09-08T21:33:21.233Z
Learning: When reviewing PRs that add new documentation files (like user_guide.rst), external documentation URLs (e.g., Read the Docs links) may return 404 during review because they only become available after the PR is merged and documentation is rebuilt. This is expected behavior and not an error.

Applied to files:

README.md

📚 Learning: 2025-08-29T20:21:30.088Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-44
Timestamp: 2025-08-29T20:21:30.088Z
Learning: boom-boot 1.6.7 is not yet released as of August 2025, so version requirements should not reference it until the actual release is made, even though snapm-0.5.0 is planned to depend on it.

Applied to files:

README.md

📚 Learning: 2025-08-25T17:19:19.563Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

README.md

📚 Learning: 2025-08-26T15:22:27.918Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#383
File: virt_tests/testvm.py:493-499
Timestamp: 2025-08-26T15:22:27.918Z
Learning: In the snapm virt_tests testing workflow, the target distributions (Fedora 41/42, CentOS Stream 9/10) all provide the bare `pip` executable when python3-pip is installed, so using `pip install` instead of `python3 -m pip install` is acceptable and works reliably across these specific distributions.

Applied to files:

CONTRIBUTING.md

📚 Learning: 2025-08-27T20:27:47.299Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-27T20:27:47.299Z
Learning: For systemd unit installation in snapm.spec, prefer the explicit mkdir -p + install style over install -D for better clarity and to avoid obfuscating directory creation responsibility.

Applied to files:

CONTRIBUTING.md

📚 Learning: 2025-08-27T21:01:32.324Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-27T21:01:32.324Z
Learning: In the snapm project, the design philosophy is to fail fast when expected files (like systemd units or tmpfiles.d configuration) don't exist during installation steps. This is intentional behavior - if these files are missing, it indicates a fundamental issue with the build/test setup that should abort the process immediately rather than continue with an incomplete installation.

Applied to files:

CONTRIBUTING.md

📚 Learning: 2025-08-29T20:18:44.022Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: doc/user_guide.rst:518-530
Timestamp: 2025-08-29T20:18:44.022Z
Learning: For thin volumes (lvm2-thin provider), the "Free" field represents available space in the thin pool while "Size" represents the virtual size of the volume. It's normal and expected for Free > Size in thin provisioning scenarios.

Applied to files:

doc/user_guide.rst

📚 Learning: 2025-09-09T16:06:19.848Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

Applied to files:

doc/user_guide.rst

📚 Learning: 2025-09-09T19:56:30.417Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1433-1461
Timestamp: 2025-09-09T19:56:30.417Z
Learning: In snapm JSON output, field names always use full prefixed names (e.g., "snapset_name", "snapset_status") to avoid collisions between different object types, even though prefixes may be optional in other contexts when unambiguous. This is an intentional architectural decision for consistency and conflict prevention in JSON reports.

Applied to files:

doc/user_guide.rst

⏰ 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). (8)

GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: python-basic-tests

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (2)

CONTRIBUTING.md (1)
115-121: Fix list indentation to satisfy MD007 (unordered list).

Remove the leading two spaces before list markers.
-  * The test suite requires `pytest` (and optionally `coverage`). You can
+* The test suite requires `pytest` (and optionally `coverage`). You can
If you want, I can apply the same zero-indent to the subsequent bullets for consistency.
man/man8/snapm.8 (1)
1512-1513: Missing space before “show” macro.

Readability nit in “create and show” sentence.
-The JSON format produced by the \fBcreate\fP and\fBshow\fP commands uses JSON
+The JSON format produced by the \fBcreate\fP and \fBshow\fP commands uses JSON

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 32518ee and a69e9fe.

📒 Files selected for processing (6)

CONTRIBUTING.md (5 hunks)
README.md (1 hunks)
doc/conf.py (4 hunks)
doc/index.rst (1 hunks)
doc/user_guide.rst (1 hunks)
man/man8/snapm.8 (25 hunks)

🚧 Files skipped from review as they are similar to previous changes (4)

doc/index.rst
doc/user_guide.rst
README.md
doc/conf.py

🧰 Additional context used

🧠 Learnings (20)

📓 Common learnings

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

📚 Learning: 2025-08-29T21:27:05.957Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

Applied to files:

CONTRIBUTING.md

📚 Learning: 2025-08-30T13:46:15.300Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-30T13:46:15.300Z
Learning: In snapm, boom is a hard requirement because snapm.manager._boot imports boom modules directly, and the Manager class imports and uses functionality from _boot including BootCache, create_snapset_boot_entry, and delete_snapset_boot_entry. The Manager class cannot be instantiated without boom being available.

Applied to files:

CONTRIBUTING.md

📚 Learning: 2025-08-30T14:13:38.717Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-38
Timestamp: 2025-08-30T14:13:38.717Z
Learning: The boom-boot package structure: `boom-boot` is the leaf package containing the binaries in all distributions, while `python3-boom` contains only the Python modules. For installation instructions, `boom-boot` is the correct package name to recommend.

Applied to files:

CONTRIBUTING.md

📚 Learning: 2025-08-26T15:22:27.918Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#383
File: virt_tests/testvm.py:493-499
Timestamp: 2025-08-26T15:22:27.918Z
Learning: In the snapm virt_tests testing workflow, the target distributions (Fedora 41/42, CentOS Stream 9/10) all provide the bare `pip` executable when python3-pip is installed, so using `pip install` instead of `python3 -m pip install` is acceptable and works reliably across these specific distributions.

Applied to files:

CONTRIBUTING.md

📚 Learning: 2025-08-27T20:27:47.299Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-27T20:27:47.299Z
Learning: For systemd unit installation in snapm.spec, prefer the explicit mkdir -p + install style over install -D for better clarity and to avoid obfuscating directory creation responsibility.

Applied to files:

CONTRIBUTING.md

📚 Learning: 2025-08-27T21:01:32.324Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-27T21:01:32.324Z
Learning: In the snapm project, the design philosophy is to fail fast when expected files (like systemd units or tmpfiles.d configuration) don't exist during installation steps. This is intentional behavior - if these files are missing, it indicates a fundamental issue with the build/test setup that should abort the process immediately rather than continue with an incomplete installation.

Applied to files:

CONTRIBUTING.md

📚 Learning: 2025-09-09T19:56:30.429Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1433-1461
Timestamp: 2025-09-09T19:56:30.429Z
Learning: In snapm JSON output, field names always use full prefixed names (e.g., "snapset_name", "snapset_status") to avoid collisions between different object types, even though prefixes may be optional in other contexts when unambiguous. This is an intentional architectural decision for consistency and conflict prevention in JSON reports.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T19:34:08.396Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-22T19:34:08.396Z
Learning: The snapshotmanager organization (including snapm and boom projects) uses "SHORTMONTH DAY YEAR" format for man page headers. This format has been used by boom since 2017. Organizational consistency within snapshotmanager is prioritized over cross-project consistency when it comes to formatting standards.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-24T16:09:15.238Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:496-505
Timestamp: 2025-08-24T16:09:15.238Z
Learning: In UNIX man pages for snapm, metavars (value placeholders) should be lowercase except for UUID which should be uppercase. For example, use `schedule_name` not `SCHEDULE_NAME`, but use `UUID` not `uuid`.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-09-09T19:26:01.116Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1434-1462
Timestamp: 2025-09-09T19:26:01.116Z
Learning: The `list` command JSON format in snapm uses a wrapper object structure with a type-specific key (e.g., "Snapsets", "Snapshots") that maps to an array of objects, rather than a direct JSON array. This design provides flexibility for future mixed-object-type reports and follows patterns similar to lvm2's JSON reports. For example: `{"Snapsets": [{"name": "backup", ...}, {"name": "before-upgrade", ...}]}`.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-09-09T16:06:19.848Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-20T21:09:39.353Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-20T21:09:39.353Z
Learning: In the snapm codebase, JSON output formatting follows a pattern: use `to_dict()+dumps()` for operations returning multiple objects (like list commands) to properly construct JSON arrays, and use `object.json()` for operations returning single objects (like create commands). This avoids the complexity of assembling multiple JSON object strings into valid JSON lists.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-25T17:19:19.563Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T18:27:31.307Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-22T18:27:31.307Z
Learning: The snapm project only supports GNU Groff markup for man pages and does not support other troff toolchains. All man page formatting should follow GNU Groff standards and practices.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-26T13:22:28.829Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-26T13:22:28.829Z
Learning: In the snapm project's man pages, single `.` lines are intentional spacers used to separate and improve readability between macro definitions and section headers like `.SH`/`.SS`. These should not be flagged as formatting errors or suggested for removal.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-23T22:16:04.019Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:11-12
Timestamp: 2025-08-23T22:16:04.019Z
Learning: In man pages for the snapm project, prefer backslash (`\`) continuations over multiple macro invocations when breaking long command lists across lines, as this maintains a single macro invocation while being readable in 80x25 terminals.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-25T01:56:44.312Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#434
File: man/man8/snapm.8:1072-1073
Timestamp: 2025-08-25T01:56:44.312Z
Learning: In the snapm project man pages, raw backslash (`\`) at end-of-line in .EX example blocks is intentionally used for roff input continuation to fit long command examples within line length constraints, rather than displaying literal backslashes.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-25T01:33:17.496Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#434
File: man/man8/snapm.8:15-26
Timestamp: 2025-08-25T01:33:17.496Z
Learning: For man page URL macro fallbacks in the snapm project, man2html compatibility requires simple macro definitions. Complex approaches using string storage don't render properly in man2html. The working approach is to define UR as `\\$2 \(la\\$1\(ra` (link_text <URL>) and UE as just a line break, with both macros defined independently rather than nested in conditionals.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T16:15:49.986Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:128-133
Timestamp: 2025-08-22T16:15:49.986Z
Learning: In UNIX systems, command line arguments should remain lowercase (e.g., --uuid, --snapshot-uuid) following standard UNIX conventions. Only the metavar (value placeholder) should be uppercase (e.g., UUID). This is different from Windows conventions. Command line option names themselves should never be changed to uppercase.

Applied to files:

man/man8/snapm.8

🪛 markdownlint-cli2 (0.17.2)

CONTRIBUTING.md

115-115: Unordered list indentation
Expected: 0; Actual: 2

(MD007, ul-indent)

⏰ 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). (24)

GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: virt_tests (uefi, lvm, centos-stream9)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: virt_tests (bios, lvm, fedora42)
GitHub Check: virt_tests (bios, lvm-thin, fedora41)
GitHub Check: virt_tests (bios, lvm, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
GitHub Check: virt_tests (uefi, lvm, fedora42)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
GitHub Check: virt_tests (bios, lvm, fedora41)
GitHub Check: virt_tests (uefi, lvm, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm, centos-stream10)
GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
GitHub Check: python-basic-tests

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (7)

CONTRIBUTING.md (5)
46-58: Add a short note about privileges for dnf/builddep steps.

These commands require root. A quick “run as root or prefix with sudo” avoids confusion for new contributors.
 ```bash
 dnf builddep boom-boot snapm
+Note: Run as root (or prefix commands with sudo) on systems where your user
+is not privileged.
---

`57-65`: **Clarify privilege expectations for boom-boot install and venv usage.**

Mention that installing distro packages (boom-boot) typically needs root, while the venv is for Python deps.


```diff
-Install ``boom-boot`` first, either from distribution packages or using
+Install ``boom-boot`` first (typically requires root if using distribution
+packages), or install it using
 the manual installation instructions from the project README.md.
115-118: Fix markdown list indentation (MD007).

Unindent list items by two spaces to satisfy markdownlint and keep rendering consistent.
-  * The test suite requires `pytest` (and optionally `coverage`). You can
+* The test suite requires `pytest` (and optionally `coverage`). You can
126-129: Call out that copying config and systemd units requires root.

Minor clarity tweak to avoid permission errors.
-From the project root directory, run:
+From the project root directory, run (as root):
181-185: Optional: hint how to install build tools for Sphinx if missing.

Small UX boost for first-time contributors who hit missing make/sphinx locally.
 ```bash
 python3 -m pip install -r requirements.txt
 make -C doc html
 xdg-open doc/_build/html/index.html
+If make or Sphinx are missing, install your distro’s build tools (e.g.,
+dnf install make) and retry.
</blockquote></details>
<details>
<summary>README.md (2)</summary><blockquote>

`32-34`: **Consider simplifying RPM install command.**

If the RPM spec already Requires boom-boot, “dnf install snapm” should pull it in; listing both is harmless but redundant. Your call.

---

`84-96`: **Nice GRUB note; add a one-liner for BLS systems (optional).**

Some Fedora/RHEL users use BLS menus without visible GRUB menu; hint they may need to hold/press a key to show it.


```diff
-- Reboot and select the snapshot boot entry from the GRUB menu
+- Reboot and select the snapshot boot entry from the GRUB menu
+  (on BLS-based systems the menu may be hidden; hold/press the configured key
+  to show it)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a69e9fe and ca7745b.

📒 Files selected for processing (4)

CONTRIBUTING.md (5 hunks)
README.md (1 hunks)
doc/user_guide.rst (1 hunks)
man/man8/snapm.8 (25 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

doc/user_guide.rst

🧰 Additional context used

🧠 Learnings (22)

📓 Common learnings

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

📚 Learning: 2025-08-30T13:46:15.300Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-30T13:46:15.300Z
Learning: In snapm, boom is a hard requirement because snapm.manager._boot imports boom modules directly, and the Manager class imports and uses functionality from _boot including BootCache, create_snapset_boot_entry, and delete_snapset_boot_entry. The Manager class cannot be instantiated without boom being available.

Applied to files:

README.md
CONTRIBUTING.md

📚 Learning: 2025-08-29T21:27:05.957Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:0-0
Timestamp: 2025-08-29T21:27:05.957Z
Learning: Issue #401 (boom dependency documentation update) was removed from PR #454 and will be handled in a separate follow-up PR, as boom-boot 1.6.7 is not yet released.

Applied to files:

README.md
CONTRIBUTING.md

📚 Learning: 2025-08-30T14:13:38.717Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-38
Timestamp: 2025-08-30T14:13:38.717Z
Learning: The boom-boot package structure: `boom-boot` is the leaf package containing the binaries in all distributions, while `python3-boom` contains only the Python modules. For installation instructions, `boom-boot` is the correct package name to recommend.

Applied to files:

README.md

📚 Learning: 2025-09-08T21:33:21.233Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:166-170
Timestamp: 2025-09-08T21:33:21.233Z
Learning: When reviewing PRs that add new documentation files (like user_guide.rst), external documentation URLs (e.g., Read the Docs links) may return 404 during review because they only become available after the PR is merged and documentation is rebuilt. This is expected behavior and not an error.

Applied to files:

README.md

📚 Learning: 2025-08-29T20:21:30.088Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: README.md:36-44
Timestamp: 2025-08-29T20:21:30.088Z
Learning: boom-boot 1.6.7 is not yet released as of August 2025, so version requirements should not reference it until the actual release is made, even though snapm-0.5.0 is planned to depend on it.

Applied to files:

README.md

📚 Learning: 2025-08-25T17:19:19.563Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

README.md
man/man8/snapm.8

📚 Learning: 2025-08-26T15:22:27.918Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#383
File: virt_tests/testvm.py:493-499
Timestamp: 2025-08-26T15:22:27.918Z
Learning: In the snapm virt_tests testing workflow, the target distributions (Fedora 41/42, CentOS Stream 9/10) all provide the bare `pip` executable when python3-pip is installed, so using `pip install` instead of `python3 -m pip install` is acceptable and works reliably across these specific distributions.

Applied to files:

CONTRIBUTING.md

📚 Learning: 2025-08-27T20:27:47.299Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-27T20:27:47.299Z
Learning: For systemd unit installation in snapm.spec, prefer the explicit mkdir -p + install style over install -D for better clarity and to avoid obfuscating directory creation responsibility.

Applied to files:

CONTRIBUTING.md

📚 Learning: 2025-08-27T21:01:32.324Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-27T21:01:32.324Z
Learning: In the snapm project, the design philosophy is to fail fast when expected files (like systemd units or tmpfiles.d configuration) don't exist during installation steps. This is intentional behavior - if these files are missing, it indicates a fundamental issue with the build/test setup that should abort the process immediately rather than continue with an incomplete installation.

Applied to files:

CONTRIBUTING.md

📚 Learning: 2025-09-09T19:56:30.429Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1433-1461
Timestamp: 2025-09-09T19:56:30.429Z
Learning: In snapm JSON output, field names always use full prefixed names (e.g., "snapset_name", "snapset_status") to avoid collisions between different object types, even though prefixes may be optional in other contexts when unambiguous. This is an intentional architectural decision for consistency and conflict prevention in JSON reports.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T19:34:08.396Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-22T19:34:08.396Z
Learning: The snapshotmanager organization (including snapm and boom projects) uses "SHORTMONTH DAY YEAR" format for man page headers. This format has been used by boom since 2017. Organizational consistency within snapshotmanager is prioritized over cross-project consistency when it comes to formatting standards.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-24T16:09:15.238Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:496-505
Timestamp: 2025-08-24T16:09:15.238Z
Learning: In UNIX man pages for snapm, metavars (value placeholders) should be lowercase except for UUID which should be uppercase. For example, use `schedule_name` not `SCHEDULE_NAME`, but use `UUID` not `uuid`.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-09-09T19:26:01.116Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#454
File: man/man8/snapm.8:1434-1462
Timestamp: 2025-09-09T19:26:01.116Z
Learning: The `list` command JSON format in snapm uses a wrapper object structure with a type-specific key (e.g., "Snapsets", "Snapshots") that maps to an array of objects, rather than a direct JSON array. This design provides flexibility for future mixed-object-type reports and follows patterns similar to lvm2's JSON reports. For example: `{"Snapsets": [{"name": "backup", ...}, {"name": "before-upgrade", ...}]}`.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-09-09T16:06:19.848Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-20T21:09:39.353Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-20T21:09:39.353Z
Learning: In the snapm codebase, JSON output formatting follows a pattern: use `to_dict()+dumps()` for operations returning multiple objects (like list commands) to properly construct JSON arrays, and use `object.json()` for operations returning single objects (like create commands). This avoids the complexity of assembling multiple JSON object strings into valid JSON lists.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T18:27:31.307Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-22T18:27:31.307Z
Learning: The snapm project only supports GNU Groff markup for man pages and does not support other troff toolchains. All man page formatting should follow GNU Groff standards and practices.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-26T13:22:28.829Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#0
File: :0-0
Timestamp: 2025-08-26T13:22:28.829Z
Learning: In the snapm project's man pages, single `.` lines are intentional spacers used to separate and improve readability between macro definitions and section headers like `.SH`/`.SS`. These should not be flagged as formatting errors or suggested for removal.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-23T22:16:04.019Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:11-12
Timestamp: 2025-08-23T22:16:04.019Z
Learning: In man pages for the snapm project, prefer backslash (`\`) continuations over multiple macro invocations when breaking long command lists across lines, as this maintains a single macro invocation while being readable in 80x25 terminals.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-25T01:56:44.312Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#434
File: man/man8/snapm.8:1072-1073
Timestamp: 2025-08-25T01:56:44.312Z
Learning: In the snapm project man pages, raw backslash (`\`) at end-of-line in .EX example blocks is intentionally used for roff input continuation to fit long command examples within line length constraints, rather than displaying literal backslashes.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-25T01:33:17.496Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#434
File: man/man8/snapm.8:15-26
Timestamp: 2025-08-25T01:33:17.496Z
Learning: For man page URL macro fallbacks in the snapm project, man2html compatibility requires simple macro definitions. Complex approaches using string storage don't render properly in man2html. The working approach is to define UR as `\\$2 \(la\\$1\(ra` (link_text <URL>) and UE as just a line break, with both macros defined independently rather than nested in conditionals.

Applied to files:

man/man8/snapm.8

📚 Learning: 2025-08-22T16:15:49.986Z

Learnt from: bmr-cymru
PR: snapshotmanager/snapm#400
File: man/man8/snapm.8:128-133
Timestamp: 2025-08-22T16:15:49.986Z
Learning: In UNIX systems, command line arguments should remain lowercase (e.g., --uuid, --snapshot-uuid) following standard UNIX conventions. Only the metavar (value placeholder) should be uppercase (e.g., UUID). This is different from Windows conventions. Command line option names themselves should never be changed to uppercase.

Applied to files:

man/man8/snapm.8

🪛 markdownlint-cli2 (0.17.2)

CONTRIBUTING.md

115-115: Unordered list indentation
Expected: 0; Actual: 2

(MD007, ul-indent)

⏰ 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). (24)

GitHub Check: rpm-build:fedora-43-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:centos-stream-9-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:centos-stream-10-x86_64
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
GitHub Check: virt_tests (bios, lvm-thin, fedora41)
GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
GitHub Check: virt_tests (uefi, lvm, fedora42)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
GitHub Check: virt_tests (uefi, lvm, centos-stream10)
GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
GitHub Check: virt_tests (uefi, lvm, centos-stream9)
GitHub Check: virt_tests (bios, lvm, centos-stream9)
GitHub Check: virt_tests (bios, lvm-thin, fedora42)
GitHub Check: virt_tests (uefi, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm, fedora41)
GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
GitHub Check: virt_tests (bios, lvm, centos-stream10)
GitHub Check: virt_tests (bios, lvm, fedora42)
GitHub Check: python-basic-tests

🔇 Additional comments (5)

man/man8/snapm.8 (5)

78-86: Good: report options consolidated in one macro.

The option set reads cleanly, and the rename to sort_fields is consistent with later references.

669-675: Status aggregation precedence reads clearly.

The Invalid → Reverting → Inactive ordering is unambiguous and fixes past confusion.

1259-1277: Field list additions (basename/index) are helpful.

Great addition; matches examples further down and improves discoverability via “-o help”.

1432-1475: JSON list format and prefixed keys are well documented.

The wrapper object (“Snapsets”) and prefixed key rationale are now explicit and align with the examples.

1481-1516: Show/create JSON format clarified; consistent with the CLI.

Explicitly documenting array vs single-object output and TitleCase keys is spot on.

bmr-cymru self-assigned this Aug 29, 2025

bmr-cymru added Documentation Improvements or additions to documentation UI/UX User interface and experience labels Aug 29, 2025

coderabbitai Bot reviewed Aug 29, 2025

View reviewed changes

Comment thread doc/user_guide.rst

Comment thread doc/user_guide.rst

Comment thread doc/user_guide.rst Outdated

Comment thread doc/user_guide.rst

Comment thread doc/user_guide.rst

Comment thread README.md

Comment thread README.md Outdated

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from cc55c91 to 5701b13 Compare August 29, 2025 20:23

coderabbitai Bot reviewed Aug 29, 2025

View reviewed changes

Comment thread README.md Outdated

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from 5701b13 to 17dd959 Compare August 29, 2025 20:28

coderabbitai Bot reviewed Aug 29, 2025

View reviewed changes

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from 17dd959 to ab479bf Compare August 29, 2025 20:49

coderabbitai Bot reviewed Aug 29, 2025

View reviewed changes

Comment thread README.md Outdated

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from ab479bf to 27d07d5 Compare August 30, 2025 13:02

coderabbitai Bot reviewed Aug 30, 2025

View reviewed changes

Comment thread README.md Outdated

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from 27d07d5 to dd38256 Compare August 30, 2025 13:14

coderabbitai Bot reviewed Aug 30, 2025

View reviewed changes

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from dd38256 to b380fff Compare August 30, 2025 13:48

coderabbitai Bot reviewed Aug 30, 2025

View reviewed changes

Comment thread README.md

Comment thread README.md Outdated

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from b380fff to de8af90 Compare August 30, 2025 14:15

coderabbitai Bot reviewed Aug 30, 2025

View reviewed changes

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from de8af90 to 383ae8f Compare August 30, 2025 14:33

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from e942689 to f4a9ebd Compare September 9, 2025 19:43

coderabbitai Bot reviewed Sep 9, 2025

View reviewed changes

Comment thread man/man8/snapm.8

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from f4a9ebd to 6347a1a Compare September 9, 2025 20:22

coderabbitai Bot reviewed Sep 9, 2025

View reviewed changes

Comment thread man/man8/snapm.8

Comment thread README.md

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from 6347a1a to 3ada8b6 Compare September 9, 2025 20:48

coderabbitai Bot reviewed Sep 9, 2025

View reviewed changes

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from 3ada8b6 to 8adaeda Compare September 9, 2025 22:04

coderabbitai Bot reviewed Sep 9, 2025

View reviewed changes

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from 8adaeda to 121c226 Compare September 9, 2025 23:07

coderabbitai Bot reviewed Sep 9, 2025

View reviewed changes

Comment thread CONTRIBUTING.md

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from 537a2d9 to 67be97e Compare September 10, 2025 01:36

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from 67be97e to 8d5a331 Compare September 10, 2025 04:12

coderabbitai Bot reviewed Sep 10, 2025

View reviewed changes

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from 8d5a331 to 32518ee Compare September 10, 2025 18:35

coderabbitai Bot reviewed Sep 10, 2025

View reviewed changes

Comment thread doc/user_guide.rst

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from 32518ee to a69e9fe Compare September 10, 2025 19:54

coderabbitai Bot reviewed Sep 10, 2025

View reviewed changes

Comment thread CONTRIBUTING.md

Comment thread man/man8/snapm.8

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from a69e9fe to ca7745b Compare September 10, 2025 20:08

coderabbitai Bot reviewed Sep 10, 2025

View reviewed changes

bmr-cymru closed this Sep 10, 2025

bmr-cymru force-pushed the bmr-update-readme-user-guide branch from ca7745b to 1a720a7 Compare September 10, 2025 20:59

Conversation

bmr-cymru commented Aug 29, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related issues

Possibly related PRs

Pre-merge checks (3 passed, 1 warning, 1 inconclusive)

Poem

Uh oh!

packit-as-a-service Bot commented Aug 29, 2025

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bmr-cymru commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

bmr-cymru commented Aug 29, 2025

Uh oh!

coderabbitai Bot commented Aug 29, 2025

Uh oh!

bmr-cymru commented Aug 29, 2025

Uh oh!

coderabbitai Bot commented Aug 29, 2025

Uh oh!

bmr-cymru commented Aug 29, 2025

Uh oh!

coderabbitai Bot commented Aug 29, 2025

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

bmr-cymru commented Aug 30, 2025

Uh oh!

coderabbitai Bot commented Aug 30, 2025

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

bmr-cymru commented Aug 29, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 29, 2025 •

edited

Loading

bmr-cymru commented Aug 29, 2025 •

edited

Loading

bmr-cymru commented Sep 9, 2025 •

edited

Loading