Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions docs/copilot-pr-reviews/pr-2194-copilot-suggestions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
semantic-links:
skill-links:
- process-copilot-suggestions
related-artifacts:
- .github/skills/dev/pr-reviews/process-copilot-suggestions/SKILL.md
---

<!-- cspell:disable -->

<!-- skill-link: process-copilot-suggestions -->

# PR #2194 Copilot Suggestions Tracking

Source: Copilot PR review threads for <https://github.com/torrust/torrust-tracker/pull/2194>

Status legend:

- `action`: code/docs change applied
- `no-action`: suggestion reviewed; no code change needed
- `resolved`: thread resolved in PR

## Processing Log

- 2026-09-10 08:35 UTC: Started processing suggestions (three unresolved threads, all on `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md`, the only file the pull request adds).
- 2026-09-10 08:38 UTC: Recomputed all three claims against `develop` at `89d45145` before deciding any of them. The path filters were read from the `on:` blocks of both workflows, the inserted string was measured and the four line lengths recomputed from the workflow file itself, and the manual-evidence requirement was read from the `create-issue` skill. All three claims hold.
- 2026-09-10 08:44 UTC: Accepted all three and applied them in commit `b13e062b`: the "every push" claims are qualified by the path filters, the line-length arithmetic is corrected to 29 characters and 213, 182, 180, 185, and manual verification now produces an issue-local `manual-verification-evidence.md`.
- 2026-09-10 08:46 UTC: Gated commit `b13e062b` on the build server: `git diff --check` over the branch range against `develop` reports no whitespace defects, and `linter all` exits `0` in 18.2 s with markdown, link, YAML, TOML, spelling, clippy, formatting and shell checks all green.
- 2026-09-10 09:05 UTC: Pushed the two commits and replied on each thread with its decision and evidence, resolving each immediately after its reply, and posted a summary comment naming this record - <https://github.com/torrust/torrust-tracker/pull/2194#issuecomment-5615866135>
- 2026-09-10 09:05 UTC: Re-fetched the pull request's review threads and confirmed all three are resolved and none remains open.

## Suggestions

| # | Thread ID | Path | URL | Suggestion Summary | Decision | Reply URL | Status | Thread State |
| --- | ----------------------- | -------------------------------------------------------- | ----------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- | ------- | ------------ |
| 1 | `PRRT_kwDOGp2yqc6g_vID` | `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` | <https://github.com/torrust/torrust-tracker/pull/2194#discussion_r3977179870> | Both workflows ignore pushes whose changed files are only Markdown or `project-words.txt`, so the job does not fail on literally every feature-branch push. | action — commit `b13e062b`. The premise checks out in both workflows' `on:` blocks at `89d45145`: each carries `paths-ignore` for `**/*.md` and `project-words.txt` on `push` and on `pull_request`, and a push whose changed files all match starts no run, so there is no job to fail. The Background paragraph now states the qualification and its cause, the title drops the quantifier, and the second flagged claim, that `container.yaml` proves the invocation form on every push to `develop`, is qualified the same way. The `container.yaml` half of the report is in fact stronger than stated: that workflow's `push` trigger is also restricted to `develop`, `main` and `releases/**/*`, so it never runs on a feature-branch push at all. | <https://github.com/torrust/torrust-tracker/pull/2194#discussion_r3977287374> | DONE | RESOLVED |
| 2 | `PRRT_kwDOGp2yqc6g_vIX` | `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` | <https://github.com/torrust/torrust-tracker/pull/2194#discussion_r3977179909> | The character arithmetic is off by three: the inserted reference form adds 29 characters, so the lengths become 213, 182, 180 and 185. | action — commit `b13e062b`. The reported numbers are exactly right. the reference form, `-p torrust-tracker-e2e-tools` followed by the separating space, measures 29 characters, and the four lines of `.github/workflows/testing.yaml` at `89d45145` measure 184, 153, 151 and 156, so they land at 213, 182, 180 and 185. The specification said 26 and 210, 179, 177, 182. The conclusion the numbers support is unaffected, because 213 still exceeds the 200-character limit in `.yamllint-ci.yml` and the other three still fall inside it, so the `>-` block scalar on the first step stands. The same wrong figure in the pull request description is corrected alongside. | <https://github.com/torrust/torrust-tracker/pull/2194#discussion_r3977287904> | DONE | RESOLVED |
| 3 | `PRRT_kwDOGp2yqc6g_vIh` | `docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md` | <https://github.com/torrust/torrust-tracker/pull/2194#discussion_r3977179934> | `create-issue` requires manual-verification evidence in an issue-local `manual-verification-evidence.md`; a passing run URL alone does not preserve it. | action — commit `b13e062b`. The requirement is in the `create-issue` skill verbatim and in the issue template's own rows, and this specification did not meet it: it asked for a run URL and nothing else. T2, the in-scope verification bullet, the T2 commit point, the workflow checkpoint, the manual-verification acceptance criterion, both manual scenarios and the AC2 evidence row now name the issue-local file and the section each scenario writes, with the run URL carried there as the reference to the run rather than standing in for the evidence. The template is added to the specification's related artifacts. | <https://github.com/torrust/torrust-tracker/pull/2194#discussion_r3977288378> | DONE | RESOLVED |

## Notes

- Keep this file as an audit log of review handling for the PR.
- Reply on every PR suggestion thread before resolving it so the decision is visible to reviewers. All three threads were replied to before being resolved, one at a time, and the round is summarised in a pull request comment that names this record - <https://github.com/torrust/torrust-tracker/pull/2194#issuecomment-5615866135>
- Suggestion 1 evidence: `testing.yaml` at `89d45145` declares `on: push: paths-ignore: ["**/*.md", "project-words.txt"]` and the same list under `pull_request`; `container.yaml` declares the identical two entries under both events, and additionally restricts `push` to branches `develop`, `main` and `releases/**/*` and `pull_request` to `develop` and `main`. A push whose changed files all match a `paths-ignore` pattern does not start the workflow, so the `docker-e2e` job cannot fail on it. The specification's own evidence run remains valid: the `4bff469e` push it cites carried workflow and source changes, not documentation alone.
- Suggestion 2 evidence: `len('-p torrust-tracker-e2e-tools ')` is 29, the flag being 28 characters and the separating space one more. Lines 185, 189, 193 and 197 of `.github/workflows/testing.yaml` at `89d45145` measure 184, 153, 151 and 156 characters, which the specification already recorded correctly, so the sums are 213, 182, 180 and 185. Against `line-length: max: 200` in `.yamllint-ci.yml` only the first exceeds the limit, which is what the block-scalar requirement rests on.
- Suggestion 3 evidence: the `create-issue` skill requires "manual verification scenarios that describe real human-oriented feature use or bug reproduction, with status tracked in the spec and actual commands, output, and relevant logs recorded in issue-local `manual-verification-evidence.md`". `docs/templates/ISSUE.md` carries the same requirement in its workflow checkpoints, its acceptance criteria, its manual-scenario evidence cells and its notes, and `docs/templates/MANUAL-VERIFICATION-EVIDENCE.md` is the file's template, with sections for environment, steps performed, observed result and conclusion.
- Follow-up outside this pull request: GitHub issue #2179 still carries the unqualified title "Docker E2E job in testing.yaml fails on every feature-branch push"; the specification heading no longer does, and the issue title needs the same edit to stay in step.
- Validation: `git diff --check` over the branch range against `develop` reports no whitespace defects, and `linter all` exits `0` on the build server, first at commit `b13e062b` and then again at the branch tip carrying this record, so each commit is green on its own. The run covers this file and the specification through markdownlint, lychee, cspell, YAML, TOML and the shell checks.
Loading