Skip to content

chore(pr-reviews): land the PR #2537 review fixes that missed its merge - #2539

Merged
josecelano merged 3 commits into
torrust:developfrom
josecelano:chore/2537-review-follow-up
Oct 15, 2026
Merged

josecelano merged 3 commits into
torrust:developfrom
josecelano:chore/2537-review-follow-up

Conversation

@josecelano

Copy link
Copy Markdown
Member

Lands the three review fixes for PR #2537 that missed its merge. #2537 merged at its approved rebased head before these commits reached it. The maintainer's approval for this follow-up is #2537 (comment).

Related to #2537 and #2536.

Changes

Pre-commit and pre-push checks passed through the installed git hooks.

josecelano and others added 2 commits October 15, 2026 07:54
…ot the dictionary

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ation push

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 15, 2026 06:56
@josecelano
josecelano requested a review from a team as a code owner October 15, 2026 06:56
… merge as follow-up work

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

🟢 Approval recommended

The focused documentation changes match the approved follow-up scope, with no unresolved findings.

0 open findings

What changed in this PR

Lands the documentation and workflow fixes that missed PR #2537’s merge.

Changes:

  • Requires the commit-ID check before documentation pushes, not only after rebases.
  • Moves unreassigned from the shared dictionary to a file-local spelling exception.
  • Records F17–F19 as approved follow-up work, pending merge.
File Description
project-words.txt Removes unreassigned.
docs/​pr-reviews/​pr-2537-review/​PR-REVIEW.md Records follow-up approval, findings, and pending thread states.
.github/​skills/​dev/​git-workflow/​open-pull-request/​SKILL.md Expands when the commit-ID check is required.

🧠 Review effort: Balanced


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

@da2ce7 da2ce7 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.

Reviewed at 51ca18affd7c0cfa198dc69917fc9fca0d82a342. Recomputed from the bytes at this head, against the merge base e6b46013b; develop is ee4eb8e9c and git merge-tree against it is clean.

F17 as landed. git log -1 --format=%B e6b46013b carries the original description, so the #2537 merge commit never mentions the skill change. develop history is never rewritten, so this PR's body, which goes into its own merge commit, is the only remaining durable place for that note. The body describes the earlier change accurately against the skill at e6b46013b^2: every changed text file is scanned, all-digit ids are kept, and only ids that resolve outside develop are reported. The first F17 reply ("Fixed by editing the PR body") was untrue once the merge commit existed, and the 06:57 follow-up reply corrects it.

F18 as landed. :78-81 make the check run "before every push that changes an issue spec, evidence file, or review audit, and before pushing after a rebase". That matches the artifacts the rule at :74-77 lists. The PR body and review replies cannot be scanned by a command over changed files, and :101 still says they are a manual read. The lines are at most 100 characters, and MD013 is off.

F19 as landed. unreassigned is gone, and unpushed and unrecognised are neighbours again. The directive sits two lines below the skill-link marker, as in the PR #2423 audit (:10, :12) and the PR #2344 audit (:11, :13). At the head, git grep finds the word only in those three audits, and each carries its own directive, so no file relied on the dictionary entry.

Audit update. The validator exits 0 offline on the #2537 record (19 rows, 8 log entries). It skips URL resolution references and checks only that each Reply URL is on its own thread, so the three FOLLOW_UP/OPEN rows pass. The record matches the capture. Review 5525009643 is APPROVED at 21:38:28Z, at the rebase head, with three Nit comments. The merge came at 06:44:22Z, the replies at 06:44:50-53Z, and the follow-up replies at 06:57:02-06Z. The approval is comment 6186327939, which the Ownership field (process-pr-review :217) now holds; it is retrospective and states its ~06:52Z approval time (:125-127). FOLLOW_UP and OPEN are defined in the record, and each detail's Follow-up PR URL names #2539, as :137-140 and :236-237 ask. The rebase-head legend entry is the merged head. "Fourth review" counts the four posted reviews. The 06:58 entry precedes its commit (06:58:15Z), and nothing above it changed. The body's audit bullet matches the diff. One category deviates (F1).

Process. No audit for this PR is due yet: the only review so far is an overview with no findings, which earns no row (:51-52). Related to #2537 and #2536 fits open-pull-request :156-157 for a follow-up that closes nothing, and the title rightly carries no issue tag (AGENTS.md:394). Each commit has one well-formed trailer line and no other body, author equals committer, and git diff --check is clean. The skill's id check, run verbatim in wt-head-2539-r1, prints nothing. The chore title type is F2.

Findings. F1 and F2 (Nit), inline. None is blocking.

Checked, no finding. develop's #2535 delta touches only the #2493 spec folder, none of these three files. The review bot's overview has no inline comments to number. A few new log lines run past 100 characters, which MD013 allows.

Checks

On the loop's compute hub at this head, merge base e6b46013b (receipt server-gates-krkavec-306): pre-commit profile gate exit 0 (80 s); linter markdown, linter cspell, linter lychee exit 0; frontmatter-validator --all: 19 errors / 4 warnings at this head and 19 / 4 at e6b46013b re-measured, none in the files this PR touches; validate-audit-record.py --pr-number 2537 exit 0 against the PR #2537 audit record at this head (19 rows, 8 log entries, 0 failures). GitHub checks at this head at posting time: "Docs Lint: completed/success". At posting, develop is ee4eb8e9c.

Comment thread docs/pr-reviews/pr-2537-review/PR-REVIEW.md
Comment thread .github/skills/dev/git-workflow/open-pull-request/SKILL.md
@da2ce7

da2ce7 commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

ACK 51ca18a — F17-F19 as landed, the PR #2537 audit's follow-up rows against the capture, and the clean merge against ee4eb8e were recomputed, and only non-blocking findings stand

@josecelano josecelano self-assigned this Oct 15, 2026
@josecelano

Copy link
Copy Markdown
Member Author

ACK 51ca18a

@josecelano
josecelano merged commit 759893b into torrust:develop Oct 15, 2026
1 check passed
@josecelano

Copy link
Copy Markdown
Member Author

Maintainer approval for the review findings that were unprocessed when this PR merged: @da2ce7's review #2539 (review) was submitted at 2026-10-15 07:15:52Z, and this PR merged at 07:21:44Z with its two inline findings still unprocessed.

Both land in one loop-closing PR from develop. That PR also closes out PR #2537's F17-F19, which this PR delivered. It creates docs/pr-reviews/pr-2539-review/PR-REVIEW.md for this PR's findings. The two threads stay open until it merges.

josecelano added a commit to josecelano/torrust-tracker that referenced this pull request Oct 15, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
josecelano added a commit that referenced this pull request Oct 15, 2026
…merged

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
josecelano added a commit that referenced this pull request Oct 15, 2026
…after PR #2539 merged

e346138 docs(pr-reviews): add the PR #2539 review audit (Jose Celano)
45ebf36 docs(pr-reviews): categorize the PR #2537 F19 finding as maintainability (Jose Celano)
dbfee0a docs(pr-reviews): close the PR #2537 follow-up findings after PR #2539 merged (Jose Celano)

Pull request description:

  Closes the review loop for PR #2537 now that its follow-up PR #2539 has merged. It also processes the two findings on #2539 that were unprocessed when #2539 merged.

  Related to #2537 and #2539. Maintainer approvals for the post-merge work: #2537 (comment) (#2537) and #2539 (comment) (#2539).

  ## Changes

  - `docs/pr-reviews/pr-2537-review/PR-REVIEW.md`:
    - F17-F19 move from `FOLLOW_UP`/`OPEN` to `FIXED`/`RESOLVED`. They are re-verified on `develop`, and each row cites its new `FIXED` reply.
    - #2539's F1: F19's category changes from `other` to `maintainability`, matching the PR #2423 audit's record of the same concern.
  - `docs/pr-reviews/pr-2539-review/PR-REVIEW.md` (new): records #2539's F1 as `FIXED` by this PR and F2 (the `chore` title, now a permanent merge subject) as `NO_ACTION`, as the maintainer approved.

  #2537's three threads are resolved once this branch's audit update is pushed. #2539's two threads are resolved after this PR merges.

ACKs for top commit:
  josecelano:
    ACK e346138

Tree-SHA512: 68ef9a950aaa7d141514e4d345f0c14f0e1dc6bf88272480f277522d9392effe8b6968c72f0f187df0b644368e8d5c87ab5ad9f2facfde1aab571c28b37c1e5c
@josecelano
josecelano deleted the chore/2537-review-follow-up branch October 15, 2026 07:45
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.

3 participants