Skip to content

docs: how to sync skeleton changes into a derived module - #6

Merged
eilandert merged 2 commits into
mainfrom
docs/skeleton-sync-chapter
Aug 1, 2026
Merged

eilandert merged 2 commits into
mainfrom
docs/skeleton-sync-chapter

Conversation

@eilandert

Copy link
Copy Markdown
Member

Came out of a session that went looking for the four known drift defects in the sibling modules and found that two of them cannot exist there yet. Documentation only; no workflow, script or module code changes.

TL;DR

A module gets cloned from here once and then drifts. There was no written procedure for forwarding a later skeleton fix into it — only ci/PROMPT-standardize-module.md, which is the eight-phase, eight-PR first-time bring-up and never said so. So the small job ("port this one improvement") had only the large document to land in.

What changed

  • README gains ## Syncing skeleton changes into a derived module: establish an anchor, select one concern per PR, re-derive rather than copy, check the four drift classes, verify the gate red, record the anchor in the memory mirror, send improvements back.
  • Prompt gains a ## Scope — is this the right document? header naming the tell for an already-standardised target — ci/ layout, ci.yml as sole pull_request entry point, ci/linter/ — and pointing at the README for the smaller job.
  • Prompt Phase 2 gains the port-band rule, which was missing entirely. Test::Nginx binds TEST_NGINX_PORT, default 1984, unarbitrated; builder02 runs six slots against one network. Two jobs on the default collide and the loser dies with bind() to 127.0.0.1:1984 failed (98: Address already in use), which reads as a module regression. Documents the reference shape: per-workflow TEST_BASE_PORT (build-test.yml 19200, ci-deep.yml 19400) passed as TEST_NGINX_PORT, band proven free by ci/tools/max-port.sh.

The drift list is narrower than the earlier carry-note claimed: versions.env validation and the workflow_policy.py regex bypasses only apply to a module that already carries those files, which no sibling does. That correction is stated in the README rather than left implicit.

Testing

ci/linter/run-all.sh clean on the working tree, and again in --staged mode at commit. lint-docs-drift reports 10 workflows, all documented in README.md — the check that the badge row and CI table stay in lockstep with reality, and the one this diff could plausibly have broken. No behaviour change to test: no .c, .sh, .py or workflow file is touched.

The prompt covered the first-time bring-up and never said so, so a task that
was really 'forward one skeleton improvement' landed in an eight-phase,
eight-PR document. Splits the two: README gains a sync checklist, the prompt
gains a scope header naming the tell (ci/ layout, ci.yml as sole pull_request
entry, ci/linter/) and pointing at the README for the smaller job.

Phase 2 also gained the port-band rule. It was absent, which is how modules
ended up running prove on Test::Nginx's unarbitrated default 1984 against
builder02's six slots; the collision surfaces as a bind() failure that reads
as a module regression.
@coderabbitai

coderabbitai Bot commented Aug 1, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@eilandert, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 53 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: cb02ac59-8ec4-4e60-8274-1913a2362860

📥 Commits

Reviewing files that changed from the base of the PR and between 68b622e and 04932de.

📒 Files selected for processing (2)
  • README.md
  • ci/PROMPT-standardize-module.md

Walkthrough

The changes distinguish first-time module standardization from later skeleton synchronization. They add synchronization procedures and require unique, verified Test::Nginx port bands for prove jobs.

Changes

Module standardization and synchronization

Layer / File(s) Summary
Standardization scope and test port requirements
ci/PROMPT-standardize-module.md, README.md
The prompt defines first-time standardization scope, identifies standardized modules, and requires distinct job-level Test::Nginx port bands passed through TEST_NGINX_PORT.
Derived module synchronization
README.md
The README documents anchors, selective syncing, repository-specific re-derivation, drift checks, verification, CI and merge steps, upstreaming, and synchronization state recording.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main documentation change: syncing skeleton changes into a derived module.
Description check ✅ Passed The description accurately explains the documentation changes, their purpose, scope, and validation results.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/skeleton-sync-chapter
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch docs/skeleton-sync-chapter

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d39e7eaf-c2e6-4147-af72-a422f5ca9696

📥 Commits

Reviewing files that changed from the base of the PR and between 26cf5e9 and 68b622e.

📒 Files selected for processing (2)
  • README.md
  • ci/PROMPT-standardize-module.md

Comment thread ci/PROMPT-standardize-module.md
Comment thread README.md Outdated
Both from CodeRabbit on #6, both confirmed against the code.

max-port.sh runs AFTER prove in build-test.yml and guards the runtime fixture,
so the earlier text was wrong to describe the band as verified before binding.
The prompt now states where the check belongs, says the reference does not do
that for ci/t/, and tells a target to put it earlier rather than copy the order.
The reference's own reorder is filed as a follow-up, not done here.

The gcovr condition is the runner's gcovr major, not whether a pin exists:
--gcov-object-directory arrived in 7.0, --object-directory is accepted by both.
@eilandert

Copy link
Copy Markdown
Member Author

Both confirmed against the code, both fixed in 04932de.

Port-band contract. Correct, and the error was mine in the doc rather than a
gap between two sections. build-test.yml runs Verify this job's port band is free after the Run ci/t/ step and before the runtime suite — the step
comment's "fails BEFORE the fixture starts" is true of the runtime fixture only.
So ci/t/ binds TEST_BASE_PORT unverified today, and describing the reference
as verifying before binding was wrong.

The prompt now states where the check belongs, records that the reference does
not do it for prove, and tells a target to place it earlier rather than copy
the order. The README bullet asks for a distinct job-level band verified with
max-port.sh, not merely a TEST_NGINX_PORT that is set.

Reordering the reference's own step is a workflow change and this PR is
documentation, so it is filed as a follow-up rather than pulled in here. Mixing
them would put a CI behaviour change behind a docs review.

gcovr. Also right, and the sharper condition is the useful one: the runner's
gcovr major, not whether a pin exists. --gcov-object-directory arrived with the
7.0 prefix standardisation and --object-directory is accepted by both, so the
portable spelling is the advice worth giving a target whose runner you do not
control. Reworded to that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant