Skip to content

chore: restore pub(super) on placeholders and say title tiebreak - #2732

Merged
seamus-sloan merged 3 commits into
mainfrom
u/sloan/browse-review-nits
Oct 13, 2026
Merged

seamus-sloan merged 3 commits into
mainfrom
u/sloan/browse-review-nits

Conversation

@seamus-sloan

@seamus-sloan seamus-sloan commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Restore pub(super) on page::placeholders, per rule 05's visibility rule
  • Fix the comment saying the offline replica breaks ties on id; it uses title

Test plan

  • cargo nextest run -p omnibus-db books::page → 58 passed
  • cargo fmt --check clean; clippy reports nothing in the touched files

Version

By default a merge to main cuts a patch release. Pick the version update this PR should trigger; labels override the default:

  • Minor — add the minor version label (vX.Y.Z → vX.(Y+1).0).
  • Patch — the default; add the patch version label or leave unlabeled (vX.Y.Z → vX.Y.(Z+1)).
  • No release — add the no release label to skip cutting a release.

If both minor version and patch version are applied, minor version wins. Unlabeled PRs that only touch docs (*.md, docs/) or CI (.github/) skip the release automatically.

@seamus-sloan seamus-sloan added documentation Improvements or additions to documentation no release Skip cutting a release when this PR merges to main labels Oct 11, 2026
Copilot AI balanced review requested due to automatic review settings October 11, 2026 03:08
@seamus-sloan seamus-sloan added documentation Improvements or additions to documentation no release Skip cutting a release when this PR merges to main labels Oct 11, 2026
@seamus-sloan seamus-sloan self-assigned this Oct 11, 2026
@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

cargo-audit

No RustSec advisories found in Cargo.lock ✅

Posted by the Rust workflow's security job.

@codecov

codecov Bot commented Oct 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The documentation fixes are accurate, with only a nonblocking visibility-style concern.

1 open finding
What changed in this PR

Corrects offline-sort documentation and adjusts the SQL placeholder helper’s visibility.

Changes:

  • Updates comments to name the title tiebreak correctly.
  • Changes placeholders to pub(super).
File Description
docs/​architecture.md Corrects offline replica tiebreak documentation.
db/​src/​books/​projection.rs Corrects the projection comment.
db/​src/​books/​page.rs Changes helper visibility.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread db/src/books/page.rs
@seamus-sloan
seamus-sloan merged commit 7e89cee into main Oct 13, 2026
45 checks passed
@seamus-sloan
seamus-sloan deleted the u/sloan/browse-review-nits branch October 13, 2026 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation no release Skip cutting a release when this PR merges to main

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants