Repository navigation
docs: fix factual drift in README, PROMPT.md and tool headers - #16
Conversation
|
Warning Review limit reached
Next review available in: 46 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
WalkthroughThe pull request updates project and CI documentation. It records eleven adoption checkpoints, expanded fuzzing coverage, blocking Semgrep warnings, nginx 1.31.3 examples, and the relocated ChangesDocumentation and CI alignment
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ci/PROMPT.md (1)
19-28: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the phase model consistent across both documents.
The phase table defines
−1,0,1,2,3, and4, but the summaries say five phases and omit close-out. Define which phases the count includes, then apply the same wording in both files.
ci/PROMPT.md#L19-L28: clarify the five-phase count or update it to include all six phase labels.README.md#L296-L302: include Phase 4 close-out, or explicitly state that the summary excludes it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e619c772-556c-4b6f-9843-d43720eb8d33
📒 Files selected for processing (8)
README.mdci/PROMPT.mdci/fuzz/build.shci/fuzz/regressions/README.mdci/linter/README.mdci/tools/ci-build.shci/tools/rename-module.shci/tools/soak.sh
| # | ||
| # Needs clang with -fsanitize=fuzzer. The nginx source tree must already be at | ||
| # .build/nginx-<VER>/ (run tools/ci-build.sh first): the target links nginx's | ||
| # .build/nginx-<VER>/ (run ci/tools/ci-build.sh first): the target links nginx's |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the mode-specific build-tree path.
Line 11 still says .build/nginx-<VER>/, but this script resolves .build/nginx-<ver>-<mode> trees. Update the comment to include <MODE> or refer users to ci/tools/nginx-tree.sh.
c13e9eb to
536bf76
Compare
Proofread pass against the machinery each file describes.
README:
- quickstart built nginx 1.31.2; the pinned mainline is 1.31.3 and
1.31.2 has no sha256 in .github/versions.env, so a copy-paste build
fetched a tarball nothing verifies
- semgrep was listed as advisory in the CI table. security-scanners.yml
runs a second pass with --severity=WARNING --error, which blocks
- "eight checkpoints" x2: PROMPT.md has eleven (0-10), of which ten
land a PR -- checkpoint 0 is read-only. The prose summary also
omitted the decision seam and the depth pass entirely
- "~350 lines of C": src/ is 1073
- ci/fuzz/ described one target; fuzz_scan and fuzz_body both build
- layout tree omitted the four ci/tools/ bump+versions scripts and all
of .github/versions.env, scripts/ and actions/build-cache/
PROMPT.md:
- checkpoint count off by one, and the PR count conflated with it
- "five phases" against a table listing six (-1, 0, 1, 2, 3, 4).
Phases -1 and 4 carry no checkpoint but are still work, so the count
was wrong rather than the table
- two dangling refs to a section 6b that does not exist; the semgrep
--jobs=1 rationale they point at is 7b
ci/linter/README.md: split a run-on sentence that glued the LINT_ONLY
duplication warning to lint.yml's runner.
Tool headers: ci-build.sh, rename-module.sh, soak.sh, fuzz/build.sh and
fuzz/regressions/README.md all printed bare tools/ paths that have not
existed since the ci/ move, and the same stale 1.31.2.
536bf76 to
b503854
Compare
|
Confirmed, and it is the same drift class the PR is about — I propagated "five phases" into the README instead of counting the table. The table is right and the count was wrong: Fixed in |
1 participant
Eleven findings, one class: text that was true when written and drifted as the repo moved under it. Two of them would mislead someone acting on the file.
The two that bite
The quickstart built an unpinned nginx.
README.mdandrename-module.shboth printedci-build.sh nginx 1.31.2. The pin isNGINX_MAINLINE=1.31.3, and.github/versions.envcarries a sha256 per version string — 1.31.2 has no entry, so a copy-paste build fetches a tarball nothing verifies. That is the one place in the repo where following the docs opts you out of the digest check.semgrep was documented as advisory; it blocks. The
## CItable listedsemgrep (advisory).security-scanners.ymlruns it twice: pass 1 full-severity into the artifact, never blocking, then pass 2 with--severity=WARNING --severity=ERROR --error, which gates. The step is calledsemgrep (gate on >=WARNING).ci/PROMPT.md:515andci/linter/README.mdboth had it right — the README was the only copy that was wrong, and it is the outward-facing one.The rest
README.md:ci/PROMPT.mdis eleven (0–10) in five phases. The prose summary also enumerated nine things while silently omitting the decision seam (cp 1) and the depth pass (cp 10), which are the two newest checkpoints and the ones an adopter most needs to know exist.src/is 1073 (520 module + 413 scan + 140 header).ci/fuzz/described as one target;fuzz_scanandfuzz_bodyboth build.ci/tools/bump/versions scripts, and all of.github/versions.env,.github/scripts/and.github/actions/build-cache/— so a reader learns the pin file exists only from the Caching section, three screens later.ci/PROMPT.md:6b. There is no 6b; checkpoint 6 has no sub-parts. Thesemgrep --jobs=1rationale they point at is 7b.ci/linter/README.md: one run-on sentence that glued theLINT_ONLY-duplication warning to an unrelated fact aboutlint.yml's runner.Tool headers —
ci-build.sh,rename-module.sh,soak.sh,fuzz/build.sh,fuzz/regressions/README.md: usage lines still printed baretools/…paths from before theci/move, so every documented invocation named a path that does not exist.Testing
Docs-only, so the deterministic gate is the whole gate:
ci/linter/run-all.sh—== all linters clean ==(includeslint-docs-drift: 10 workflows, all documented, both directions).review-lint.sh --branch main— "no reviewable files in selection", as expected for a diff with no code.1.31.2and bare-tools/sweeps that found the last six sites now return empty across all tracked*.md,*.shand*.py.The four new
ci/tools/layout-tree descriptions were written from each script's own header, not inferred from its name — two of my first drafts were wrong (bump-versions.shis the umbrella that calls the other two;bump-tools.shis PyPI linter pins specifically) and are corrected here.Nothing in this PR is verified by a build: no claim in it depends on one.