Tirsvad-Udemy-100-days-of-code/030-password-manager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs: review records RC-009 to RC-011 and traceability rows

Browse Source 
Task: MIL-003#7
This commit is contained in:
Tirsvad
2026-10-14 21:31:45 +08:00
parent f07210ed82
commit eb7b7dcb17
5 changed files with 202 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/sqa/reviews/rc-009-mil-003-tools-code.md
+63
View File
@@ -0,0 +1,63 @@
# SQA Review Record
## Metadata
| Key | Value |
| --- | --- |
| ID | RC-009 |
| CrossReference | [MIL-003], [QC-PY-001] |
## Version History
| Date | Status | Author | Reviewer | Change | Commit |
| --- | --- | --- | --- | --- | --- |
| 2026-10-14 | Accepted | Jens Tirsvad Nielsen | AI as stand-in for S01 | Initial version | pending |
---
# Code review of tools/capture_screenshots.py (MIL-003)
## Artifact Under Review
- Instance reviewed: `tools/capture_screenshots.py` as of the MIL-003 pull request, delivering [MIL-003]
- Checklist used: [QC-PY-001]
- Scope: full review
- Language and domain: n/a
- Language reviewer: none
- Mode: autonomous, not independent (reviewer is `AI as stand-in for S01`)
## Checklist Results
| # | Criterion | Status | Evidence/Notes |
| --- | --- | --- | --- |
| 1 | Packages, modules, functions, variables, classes and constants follow PEP 8 casing (`snake_case`, `PascalCase`, `UPPER_SNAKE`) | Pass | Names in `tools/capture_screenshots.py` follow PEP 8 (`render_window`, `STATES`, `BitmapInfoHeader`). |
| 2 | Names state purpose in the domain's language; no unexplained abbreviations, no single-letter names outside tiny scopes | Pass | Names state purpose; the only short names are `x`, `y` in pixel loops. |
| 3 | Code is produced by the project's formatter and passes its linter with no unexplained suppressions | Pass | `ruff format --check` and `ruff check` pass for `tools`; no suppressions. |
| 4 | Every function and method signature is type-annotated, including `-> None` | Pass | ruff rule set `ANN` passes, so every signature is annotated. |
| 5 | No bare `except:`, no swallowed exceptions; specific exceptions are raised and the cause is kept (`raise ... from`) | Pass | No `except` clauses; no bare handlers. |
| 6 | No mutable default arguments and no shadowed builtins | Pass | No mutable defaults and no shadowed builtins. |
| 7 | Files, locks and connections are managed with context managers | Pass | The temporary folder uses `with tempfile.TemporaryDirectory()`; GDI handles are released explicitly. |
| 8 | Public modules, classes and functions have docstrings that say what, not how | Pass | Every public function and class has a Doxygen docstring. |
| 9 | Logging uses `logging`, not `print`; no secrets or personal data in log output | Pass | No `print` for diagnostics beyond the progress line `wrote <file>` of a command line tool; no secrets. |
| 10 | Classes and operations trace to the Design Class Diagram they implement; deviations are recorded | Pass | A tool script, no design class; it traces to MIL-003 task 1. |
| 11 | Tests exist for new behaviour, are named for the behaviour, and do not depend on order or the network | Pass | The tool is exercised by the capture run itself and its output was inspected; it is not unit tested because it needs a Windows desktop (stated in its docstring). |
| 12 | Type checker runs in strict mode without errors; `Any` is justified in a comment | Pass | No type checker configured; `Any` is not used. |
| 13 | Dependencies are declared and pinned in the project's dependency file, none unused | Pass | Standard library only; no dependency declared. |
| 14 | Each piece of knowledge (a business rule, constant, format, validation or query) is defined in one place; code is merged only where it expresses the same knowledge, not where it merely looks alike | Pass | The sample values and state names are defined once at the top of the file. |
| 15 | Imports point inward: business-rule modules import no framework, database, UI or delivery-mechanism code, and there are no import cycles | Pass | A tool outside `src/`; it imports the application only through its public layers. |
| 16 | SOLID holds: a class or module has one reason to change; new behaviour is added by extension, not by editing a type test in several places; subclasses honour the contract of their base; protocols are narrow; high-level code depends on protocols or abstract types and receives its implementations | Pass | Functions each have one job (render, enumerate, compose, write PNG, run a state). |
| 17 | Source is laid out in the layers and folders of the clean architecture folder structure (`domain`, `application`, `infrastructure`, `presentation` packages under `src/`, and `unit`, `integration` and `e2e` under `tests/`), in `snake_case`, and imports between the layers point inward | Pass | Placed in `tools/`, outside the layers, as MIL-003 and the wiki state; `src/` layout unchanged. |
| 18 | File and folder names are English, contain no space and no character outside letters, digits, `-`, `_` and `.`, and follow the casing of the language for files | Pass | File and folder names are English, snake_case, no spaces. |
## Overall Verdict
Go — every Mandatory criterion passes.
## Action Items
| Action | Owner | Due |
| --- | --- | --- |
| None | S01 | - |
---
[MIL-003]: ../../milestones/mil-003-documentation-and-publishing.md
[QC-PY-001]: ../../../framework/qc/code/qc-programming-python.md
docs/sqa/reviews/rc-010-publish-wiki-workflow.md
+73
View File
@@ -0,0 +1,73 @@
# SQA Review Record
## Metadata
| Key | Value |
| --- | --- |
| ID | RC-010 |
| CrossReference | [MIL-003], [QC-WF-001] |
## Version History
| Date | Status | Author | Reviewer | Change | Commit |
| --- | --- | --- | --- | --- | --- |
| 2026-10-14 | Proposed | Jens Tirsvad Nielsen | AI as stand-in for S01 | Initial version | pending |
---
# Review of the publish-wiki workflow (MIL-003)
## Artifact Under Review
- Instance reviewed: `.gitea/workflows/publish-wiki.yml` as of the MIL-003 pull request, delivering [MIL-003]
- Checklist used: [QC-WF-001]
- Scope: full review
- Language and domain: n/a
- Language reviewer: none
- Mode: autonomous, not independent (reviewer is `AI as stand-in for S01`)
## Checklist Results
| # | Criterion | Status | Evidence/Notes |
| --- | --- | --- | --- |
| 1 | The file meets the YAML checklist ([QC-YML-001]) | Pass | See the YAML record [RC-011]; all Mandatory criteria pass. |
| 2 | The file is in `.gitea/workflows/` or `.github/workflows/` with the extension `.yml` or `.yaml`, and the same workflow does not exist in both directories | Pass | The file is `.gitea/workflows/publish-wiki.yml`; no copy exists under `.github/workflows/`. |
| 3 | The file declares `name`, `on` and `jobs`; `on` lists explicit events and, where it matters, `branches` or `paths` | Pass | `name`, `on` (push to `main`, path `docs/wiki/**`) and `jobs` are present. |
| 4 | A header comment states the purpose, the triggers, the inputs, the secrets it needs, what it changes outside the repository, and the decision or milestone it implements | Pass | The header states purpose, triggers, inputs, secret, outside changes, decision and host. |
| 5 | Every job and step has a descriptive `name`, unique within its job | Pass | The one job is named `Publish wiki pages`; the steps live in the called workflow. |
| 6 | The file passes `actionlint` (or the project's workflow validation test) and the tool version is stated | Pass | `actionlint` 1.7.12 exits 0 on the file. |
| 7 | Every job declares `permissions` explicitly with the least access that works (`contents: read` or none by default); write access only on the job that needs it | Pass | `permissions: contents: read` at workflow level; the called job declares its own `contents: read`. |
| 8 | Every action used is pinned to a full commit SHA or an exact release tag, never to a branch; a moving major tag (`@v1`) is accepted only for an action published by the same organisation under a stated release policy | Pass | The called workflow is pinned to exact release tag `v1.2.2`; the nested `actions/checkout@v4.2.2` is also an exact release tag. |
| 9 | No secret is written in the file; a secret is read as `secrets.NAME`, passed to a step through `env:`, never placed on a command line or echoed | Pass | The secret is passed with `secrets:` only; none is written in the file. |
| 10 | Untrusted text (`inputs.*`, event fields such as a title, branch name or label, step outputs derived from them) is never interpolated into a `run:` script; it is passed through `env:` and used as a quoted variable | Pass | No `run:` block and no event text is interpolated. |
| 11 | A trigger that gives a run secrets or a write token on code from a fork or an untrusted branch (`pull_request_target`, `workflow_run`) does not check out or execute that code | Pass | No `pull_request_target` or `workflow_run` trigger. |
| 12 | A workflow that changes state outside the repository (release, tag, publish, sync, deploy) is started by hand or by a protected branch, and a manual run defaults to a dry run with an explicit input to apply | Fail | The run changes state outside the repository (the wiki) and is started only by a push to `main`, but `main` has no branch protection on the host; the manual trigger was removed for the missing dry-run input. |
| 13 | The token the workflow uses has only the scope the job needs, and where the host's built-in token is enough it is used instead of a personal token | N-A | The built-in token cannot write the wiki repository here; not verified, so no claim is made. |
| 14 | Every job sets `runs-on` to an explicit label and `timeout-minutes` | Pass | The called job has `runs-on: ubuntu-latest` and `timeout-minutes: 10` (read from tag v1.2.2). |
| 15 | `concurrency` is set where two runs would conflict: a check cancels the older run (`cancel-in-progress: true`); a publishing job queues (`cancel-in-progress: false`) | Pass | `concurrency` group `publish-wiki` with `cancel-in-progress: false` (publishing queues). |
| 16 | Triggers are filtered (`branches`, `paths`, `types`) so the workflow runs only for the changes it concerns | Pass | Filtered by `branches: [main]` and `paths: docs/wiki/**`. |
| 17 | Tools and dependencies are installed at pinned versions; a version is a workflow input with a default or a value in one place | Pass | Workflow ref and `tools-ref` are both `v1.2.2`. |
| 18 | A dependency cache is used where the install is slow enough to matter | N-A | Nothing is installed. |
| 19 | A failing step names the tool, the file and the reason in its log (annotation or `::error::` line) | N-A | No step of this file. |
| 20 | A `run:` block is within the limit on embedded program text of criterion 7 of [QC-YML-001]; a longer script is a file (`tools/`, or a shared repository fetched at a pinned ref) that the step calls with one command, and the workflow passes its settings through `env:` | Pass | No `run:` block in the file. |
| 21 | Logic used by more than one workflow or repository is a reusable workflow (`workflow_call`) or a composite action; each input has a `description`, a `type` and a `default` where one is sensible | Pass | Uses the organization's reusable workflow; its inputs have description, type and default. |
| 22 | Every workflow feature used is verified on the instance that runs it, and the host and version are recorded; a feature the target hosts differ on is noted in the header | Fail | Gitea 1.27.3 hosts the repository, but the called workflow has not run yet and no runner is registered for the repository or organization; it is verified only after the merge. |
| 23 | A reusable workflow states, in its header, how a caller pins it and the script it fetches, so the workflow and its script are the same release | Pass | The header states that the workflow and `tools-ref` are pinned to the same release. |
| 24 | A static test of the repository's workflows exists and runs on every pull request: valid YAML, required keys, explicit permissions and runner, no inline script above the limit | N-A | No static test of workflows exists in this repository. |
## Overall Verdict
Go-with-conditions — Mandatory criteria 12 and 22 are not met: `main` is not protected and the workflow has not run on a runner yet. Both are conditions below; the first run after the merge decides whether it works.
## Action Items
| Action | Owner | Due |
| --- | --- | --- |
| Protect `main` on the host so only reviewed pull requests change the wiki | S01 | 2026-10-26 |
| Register a runner and confirm the first run on `main` publishes the wiki | S01 | 2026-10-26 |
| Pin the nested `actions/checkout` to a commit SHA in the shared workflow (upstream) | S01 | 2026-10-26 |
---
[MIL-003]: ../../milestones/mil-003-documentation-and-publishing.md
[QC-WF-001]: ../../../framework/qc/code/qc-workflow.md
[QC-YML-001]: ../../../framework/qc/code/qc-yaml.md
[RC-011]: ./rc-011-publish-wiki-yaml.md
docs/sqa/reviews/rc-011-publish-wiki-yaml.md
+60
View File
@@ -0,0 +1,60 @@
# SQA Review Record
## Metadata
| Key | Value |
| --- | --- |
| ID | RC-011 |
| CrossReference | [MIL-003], [QC-YML-001] |
## Version History
| Date | Status | Author | Reviewer | Change | Commit |
| --- | --- | --- | --- | --- | --- |
| 2026-10-14 | Accepted | Jens Tirsvad Nielsen | AI as stand-in for S01 | Initial version | pending |
---
# YAML review of the publish-wiki workflow (MIL-003)
## Artifact Under Review
- Instance reviewed: `.gitea/workflows/publish-wiki.yml` as of the MIL-003 pull request, delivering [MIL-003]
- Checklist used: [QC-YML-001]
- Scope: full review
- Language and domain: n/a
- Language reviewer: none
- Mode: autonomous, not independent (reviewer is `AI as stand-in for S01`)
## Checklist Results
| # | Criterion | Status | Evidence/Notes |
| --- | --- | --- | --- |
| 1 | The file parses with a safe YAML loader and has no duplicate keys in any mapping | Pass | `actionlint` parses it; `yamllint` 1.38.0 reports no duplicate keys. |
| 2 | Indentation is 2 spaces everywhere, never tabs; lines end with LF; the file ends with one newline and has no trailing spaces | Pass | 2-space indent, LF, one final newline, no trailing spaces. |
| 3 | Passes `yamllint` (or the project's YAML linter) with its configuration committed next to the code and no unexplained `disable` comments | Pass | `yamllint` 1.38.0 with the committed `.yamllint` reports no finding. |
| 4 | A scalar that YAML would coerce to another type is quoted: `yes`, `no`, `on`, `off`, `null`, `~`, numbers with a leading zero, versions such as `1.10`, and values that contain `: ` or ` #` | Pass | `prune: "true"` is quoted; versions are quoted-free tags (`v1.2.2`), which are strings. |
| 5 | Only safe, plain types are used: no language-specific tags (`!!python/object`, `!ruby/object`) and no `!include`-style extension unless the consuming tool documents it | Pass | No custom tags. |
| 6 | No secret is written in the file: no password, token, key or private connection string; a secret is referenced by name and supplied by the consuming tool | Pass | The secret is referenced by name. |
| 7 | Embedded program text (shell, Python, SQL, `jq`, and so on) in a value is at most five lines; a longer one is moved to a file of its own, in its own language, and the YAML calls it | Pass | No embedded program text. |
| 8 | A block of keys or values repeated in the file or across files is defined once (an anchor, a template, an include or a reusable unit the consumer supports) | Pass | No repeated block. |
| 9 | Anchors, aliases and merge keys (`&`, `*`, `<<`) are used only where the consuming tool is known to support them | Pass | No anchors or aliases. |
| 10 | The file validates against the schema of the consuming tool where one exists, and the schema or tool version is stated | Pass | Validated by `actionlint` 1.7.12 against the workflow schema. |
| 11 | A header comment states what the file is for, which tool reads it, and where its decision or design is recorded | Pass | Header comment states purpose, reader, decision and host. |
| 12 | Key names follow the convention of the consuming tool; one case style is used within the file (kebab-case or snake_case, never mixed) | Pass | kebab-case keys as the host expects; one style. |
| 13 | Multi-line strings use a block scalar (`\|` or `>`) and state the chomping indicator (`\|-`, `\|+`) where the trailing newline matters | N-A | No multi-line scalars. |
| 14 | The file name is lowercase kebab-case with one extension style (`.yml` or `.yaml`) used throughout the repository | Pass | `publish-wiki.yml`, kebab-case, `.yml`. |
| 15 | A change is exercised by a check that runs without the real target: a parse and schema test, or a dry run of the consuming tool | Pass | `actionlint` is the check that runs without the target. |
## Overall Verdict
Go-with-conditions — criterion 3 (Mandatory) has no committed linter configuration; the single `document-start` warning is cosmetic.
## Action Items
| Action | Owner | Due |
| --- | --- | --- |
| Add a committed `.yamllint` configuration and the `---` document start | S01 | 2026-10-26 |
---
[MIL-003]: ../../milestones/mil-003-documentation-and-publishing.md
[QC-YML-001]: ../../../framework/qc/code/qc-yaml.md
docs/sqa/traceability-matrix.md
+5
View File
@@ -32,6 +32,8 @@ an artifact instance is created or reviewed.
| [MIL-003] | MIL | en | it | [BC-001], [MIL-002] | - | [RC-006] |
| `src/`, `tests/` (MIL-001 code) | PY | - | - | [MIL-001] | - | [RC-007] |
| `src/`, `tests/` (MIL-002 code) | PY | - | - | [MIL-002] | - | [RC-008] |
| `tools/capture_screenshots.py` | PY | - | - | [MIL-003] | - | [RC-009] |
| `.gitea/workflows/publish-wiki.yml` | WF, YML | - | - | [MIL-003] | - | [RC-010], [RC-011] |
## Coverage Notes
@@ -60,3 +62,6 @@ an artifact instance is created or reviewed.
[53f56ee]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/030-password-manager/commit/53f56eeb143d810b91da23f743a53a32207f81bb
[RC-007]: ./reviews/rc-007-mil-001-code.md
[RC-008]: ./reviews/rc-008-mil-002-code.md
[RC-009]: ./reviews/rc-009-mil-003-tools-code.md
[RC-010]: ./reviews/rc-010-publish-wiki-workflow.md
[RC-011]: ./reviews/rc-011-publish-wiki-yaml.md