|
| 1 | +# Skill: Review an open issue for TheAlgorithms/Python |
| 2 | + |
| 3 | +Triage an open issue — most often one carrying the `bug` label — **together with |
| 4 | +every pull request attached to it**, and produce a verdict a maintainer can act on |
| 5 | +without re-doing the work. The goal is a review that any triager (human or AI) can |
| 6 | +run the same way every time, and that ends in an explicit list of `Closes #NNNNN` |
| 7 | +lines the maintainer can paste into a merge commit. |
| 8 | + |
| 9 | +This repo exists to **teach visitors to fix bugs by doing**. So the default is to |
| 10 | +merge adequate existing contributor work, not to open a fresh PR that races them. |
| 11 | +Only open your own PR when no attached PR adequately solves the issue. |
| 12 | + |
| 13 | +## How to run this skill |
| 14 | + |
| 15 | +### 1. Reproduce before you trust the report |
| 16 | + |
| 17 | +- Check out current `master` and actually **run the failing case** from the issue. |
| 18 | +- If it reproduces, say so and paste the minimal reproducer (inputs → observed |
| 19 | + output, e.g. `nan` + a `RuntimeWarning`). |
| 20 | +- If you **cannot** confirm it locally — an optional dependency isn't installed, |
| 21 | + the failure needs an external service, the report is too vague — **say that |
| 22 | + explicitly rather than guessing.** "Not reproducible in a vanilla checkout" |
| 23 | + is a real, useful verdict. |
| 24 | + |
| 25 | +### 2. Classify the issue |
| 26 | + |
| 27 | +Sort each issue into one bucket and act accordingly: |
| 28 | + |
| 29 | +- **Confirmed bug with an adequate open PR** → recommend the specific PR to merge. |
| 30 | +- **Confirmed bug, no adequate PR** → open a new PR yourself (see step 5). |
| 31 | +- **Not a bug / working-as-intended** → recommend closing, and explain why (e.g. |
| 32 | + binary search returning *an* index of a duplicate is correct, not a defect). |
| 33 | +- **Feature request mislabeled as a bug** → recommend dropping the `bug` label; do |
| 34 | + **not** close it as a bug. |
| 35 | +- **Too vague / needs reporter info** → comment **on the issue itself** asking for |
| 36 | + the one concrete thing missing (a file path, a traceback, a reproducer), and note |
| 37 | + it should be closed if no answer arrives. |
| 38 | +- **Environment / optional-dependency, not an algorithm defect** → note it needs |
| 39 | + someone with that environment; it is not fixable in a vanilla checkout. |
| 40 | + |
| 41 | +### 3. Examine every attached PR — pick one, name the duplicates |
| 42 | + |
| 43 | +For a bug with several open PRs: |
| 44 | + |
| 45 | +- **Verify each fix's doctest values numerically** against a trusted reference |
| 46 | + (`scipy`, `numpy.linalg`, `geopy`, or a brute-force check over random inputs). |
| 47 | + Don't take a doctest's word for it — confirm the number. |
| 48 | +- Prefer the **first-in PR with the smallest correct diff** that follows repo |
| 49 | + convention (doctests over new test files, `raise` over `assert`, no unrelated |
| 50 | + reindentation or churn of clean doctests into floating-point noise). |
| 51 | +- **Reject out-of-scope "fixes"** — e.g. swapping a real divide-by-zero for a |
| 52 | + `1e-15` magic constant makes results implementation-defined. Fix the issue, not |
| 53 | + more. |
| 54 | +- Identify the byte-for-byte **duplicates** so the maintainer can close them with |
| 55 | + thanks in the same action. |
| 56 | + |
| 57 | +### 4. Emit the verdict — fixed output shape |
| 58 | + |
| 59 | +Post one comment that a maintainer can act on directly. For each issue give a |
| 60 | +one-line rationale and the exact autoclose line. Two formatting rules matter a lot |
| 61 | +to human maintainers scanning the thread: |
| 62 | + |
| 63 | +- **Start every line that references an issue or PR with a Markdown list marker |
| 64 | + (`-` or `* `).** GitHub-flavored Markdown only autolinks `#NNNNN` inside a list, |
| 65 | + and those autolinks are colored — **purple = merged, red = closed, green = open** — |
| 66 | + so the maintainer can see merge/close progress at a glance. A bare `#14813` at the |
| 67 | + start of a line does not autolink. |
| 68 | +- **Merging a PR that closes an issue also closes the issue's other open PRs**, so |
| 69 | + fold the duplicates into one autoclose statement rather than listing the winner |
| 70 | + alone: |
| 71 | + |
| 72 | + ```text |
| 73 | + * #14813 — 3x3 inverse returns the transpose → merge #14821 |
| 74 | + (Closes #14813, Closes #14840, Closes #15045) |
| 75 | + ``` |
| 76 | + |
| 77 | +Recommended shape: |
| 78 | + |
| 79 | +```text |
| 80 | +## Confirmed bugs with an adequate open PR — merge |
| 81 | +* #<issue> — <one-line what/why> → merge #<pr> (Closes #<issue>, Closes #<dupe>, …) |
| 82 | +
|
| 83 | +## Not a bug / not merge-ready |
| 84 | +* #<issue> — <feature-request → relabel | needs cleanup | needs a human call> |
| 85 | +
|
| 86 | +## Summary — lines to add |
| 87 | +* Closes #<issue> → #<pr> |
| 88 | +``` |
| 89 | + |
| 90 | +### 5. When you must open your own PR |
| 91 | + |
| 92 | +If no attached PR is adequate, open one — but let the contributors keep the credit |
| 93 | +where their work was close. Use autoclose keywords in the **commit message body** |
| 94 | +so the merge closes the issue *and* the superseded PRs: |
| 95 | + |
| 96 | +```text |
| 97 | +Fixes #12233 |
| 98 | +Fixes #12262 |
| 99 | +Fixes #13635 |
| 100 | +``` |
| 101 | + |
| 102 | +Follow the [`new-pull-request`](../new-pull-request/SKILL.md) skill for the |
| 103 | +mechanics (synced `master`, named branch, checked description box, untouched |
| 104 | +`uv.lock`). |
| 105 | + |
| 106 | +## Tone |
| 107 | + |
| 108 | +Be specific and kind. Name the exact PR and the exact reason, credit the |
| 109 | +contributor by handle, and offer runners-up detailed feedback so they learn — the |
| 110 | +point of the repo is that people come back and fix the next one. |
0 commit comments