Add per-library code-review scaffolding for the ZB.MOM.WW.* shared libs
Adapts the code-reviews convention (process, README generator, template) from the ScadaBridge app model (per-src/-module, Akka conventions) to scadaproj's reality: six shared libraries reviewed against their components/ specs. - REVIEW-PROCESS.md: review unit is a library; library->component-spec mapping; checklist re-targeted for reusable .NET libs (public API/semver, packaging & dependency hygiene, spec/shared-contract adherence) instead of actor/supervision. - _template/findings.md: library/packages/component-spec/shared-contract header. - regen-readme.py: per-library prose, data-driven Summary, '-' for unreviewed. - Seed Auth/Theme/Health/Telemetry/Configuration/Audit findings stubs (0 findings). - README.md generated; --check passes.
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,36 @@
|
||||
# Code Review — Audit
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Library | `ZB.MOM.WW.Audit/` |
|
||||
| Packages | `ZB.MOM.WW.Audit` |
|
||||
| Component spec | `components/audit/spec/SPEC.md` |
|
||||
| Shared contract | `components/audit/shared-contract/ZB.MOM.WW.Audit.md` |
|
||||
| Status | Not yet reviewed |
|
||||
| Last reviewed | — |
|
||||
| Reviewer | — |
|
||||
| Commit reviewed | — |
|
||||
| Open findings | 0 |
|
||||
|
||||
## Summary
|
||||
|
||||
_Not yet reviewed._
|
||||
|
||||
## Checklist coverage
|
||||
|
||||
| # | Category | Examined | Notes |
|
||||
|---|----------|----------|-------|
|
||||
| 1 | Correctness & logic bugs | ☐ | |
|
||||
| 2 | Public API surface & compatibility | ☐ | |
|
||||
| 3 | Concurrency & thread safety | ☐ | |
|
||||
| 4 | Error handling & resilience | ☐ | |
|
||||
| 5 | Security & secret handling | ☐ | |
|
||||
| 6 | Performance & resource management | ☐ | |
|
||||
| 7 | Spec & shared-contract adherence | ☐ | |
|
||||
| 8 | Packaging, dependencies & project layout | ☐ | |
|
||||
| 9 | Testing coverage | ☐ | |
|
||||
| 10 | Documentation & XML docs | ☐ | |
|
||||
|
||||
## Findings
|
||||
|
||||
_No findings recorded yet._
|
||||
@@ -0,0 +1,36 @@
|
||||
# Code Review — Auth
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Library | `ZB.MOM.WW.Auth/` |
|
||||
| Packages | `ZB.MOM.WW.Auth.Abstractions`, `ZB.MOM.WW.Auth.Ldap`, `ZB.MOM.WW.Auth.ApiKeys`, `ZB.MOM.WW.Auth.AspNetCore` |
|
||||
| Component spec | `components/auth/spec/SPEC.md` |
|
||||
| Shared contract | `components/auth/shared-contract/ZB.MOM.WW.Auth.md` |
|
||||
| Status | Not yet reviewed |
|
||||
| Last reviewed | — |
|
||||
| Reviewer | — |
|
||||
| Commit reviewed | — |
|
||||
| Open findings | 0 |
|
||||
|
||||
## Summary
|
||||
|
||||
_Not yet reviewed._
|
||||
|
||||
## Checklist coverage
|
||||
|
||||
| # | Category | Examined | Notes |
|
||||
|---|----------|----------|-------|
|
||||
| 1 | Correctness & logic bugs | ☐ | |
|
||||
| 2 | Public API surface & compatibility | ☐ | |
|
||||
| 3 | Concurrency & thread safety | ☐ | |
|
||||
| 4 | Error handling & resilience | ☐ | |
|
||||
| 5 | Security & secret handling | ☐ | |
|
||||
| 6 | Performance & resource management | ☐ | |
|
||||
| 7 | Spec & shared-contract adherence | ☐ | |
|
||||
| 8 | Packaging, dependencies & project layout | ☐ | |
|
||||
| 9 | Testing coverage | ☐ | |
|
||||
| 10 | Documentation & XML docs | ☐ | |
|
||||
|
||||
## Findings
|
||||
|
||||
_No findings recorded yet._
|
||||
@@ -0,0 +1,36 @@
|
||||
# Code Review — Configuration
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Library | `ZB.MOM.WW.Configuration/` |
|
||||
| Packages | `ZB.MOM.WW.Configuration` |
|
||||
| Component spec | `components/configuration/spec/SPEC.md` |
|
||||
| Shared contract | `components/configuration/shared-contract/ZB.MOM.WW.Configuration.md` |
|
||||
| Status | Not yet reviewed |
|
||||
| Last reviewed | — |
|
||||
| Reviewer | — |
|
||||
| Commit reviewed | — |
|
||||
| Open findings | 0 |
|
||||
|
||||
## Summary
|
||||
|
||||
_Not yet reviewed._
|
||||
|
||||
## Checklist coverage
|
||||
|
||||
| # | Category | Examined | Notes |
|
||||
|---|----------|----------|-------|
|
||||
| 1 | Correctness & logic bugs | ☐ | |
|
||||
| 2 | Public API surface & compatibility | ☐ | |
|
||||
| 3 | Concurrency & thread safety | ☐ | |
|
||||
| 4 | Error handling & resilience | ☐ | |
|
||||
| 5 | Security & secret handling | ☐ | |
|
||||
| 6 | Performance & resource management | ☐ | |
|
||||
| 7 | Spec & shared-contract adherence | ☐ | |
|
||||
| 8 | Packaging, dependencies & project layout | ☐ | |
|
||||
| 9 | Testing coverage | ☐ | |
|
||||
| 10 | Documentation & XML docs | ☐ | |
|
||||
|
||||
## Findings
|
||||
|
||||
_No findings recorded yet._
|
||||
@@ -0,0 +1,36 @@
|
||||
# Code Review — Health
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Library | `ZB.MOM.WW.Health/` |
|
||||
| Packages | `ZB.MOM.WW.Health`, `ZB.MOM.WW.Health.Akka`, `ZB.MOM.WW.Health.EntityFrameworkCore` |
|
||||
| Component spec | `components/health/spec/SPEC.md` |
|
||||
| Shared contract | `components/health/shared-contract/ZB.MOM.WW.Health.md` |
|
||||
| Status | Not yet reviewed |
|
||||
| Last reviewed | — |
|
||||
| Reviewer | — |
|
||||
| Commit reviewed | — |
|
||||
| Open findings | 0 |
|
||||
|
||||
## Summary
|
||||
|
||||
_Not yet reviewed._
|
||||
|
||||
## Checklist coverage
|
||||
|
||||
| # | Category | Examined | Notes |
|
||||
|---|----------|----------|-------|
|
||||
| 1 | Correctness & logic bugs | ☐ | |
|
||||
| 2 | Public API surface & compatibility | ☐ | |
|
||||
| 3 | Concurrency & thread safety | ☐ | |
|
||||
| 4 | Error handling & resilience | ☐ | |
|
||||
| 5 | Security & secret handling | ☐ | |
|
||||
| 6 | Performance & resource management | ☐ | |
|
||||
| 7 | Spec & shared-contract adherence | ☐ | |
|
||||
| 8 | Packaging, dependencies & project layout | ☐ | |
|
||||
| 9 | Testing coverage | ☐ | |
|
||||
| 10 | Documentation & XML docs | ☐ | |
|
||||
|
||||
## Findings
|
||||
|
||||
_No findings recorded yet._
|
||||
@@ -0,0 +1,80 @@
|
||||
# Code Reviews
|
||||
|
||||
Comprehensive, per-library code reviews of the `ZB.MOM.WW.*` shared libraries hosted
|
||||
in this repo. Each library (one self-contained `.slnx` at the repo root) has its own
|
||||
folder containing a `findings.md`. This README is the aggregated index — the single
|
||||
place to see all outstanding work.
|
||||
|
||||
> Generated by `regen-readme.py` from the per-library `findings.md` files. Do not
|
||||
> edit by hand — edit the findings files and re-run the script.
|
||||
|
||||
## How it works
|
||||
|
||||
- Reviews are performed one library at a time against a fixed checklist.
|
||||
- Each library is reviewed against its normalized component spec under `components/`.
|
||||
- Every finding is recorded in the library's `findings.md` with a severity and status.
|
||||
- Findings are **never deleted** — they are closed by changing their status, keeping
|
||||
a full audit trail.
|
||||
- This README aggregates every **pending** finding (`Open` / `In Progress`) across all
|
||||
libraries.
|
||||
|
||||
See **[REVIEW-PROCESS.md](REVIEW-PROCESS.md)** for the full procedure: the review
|
||||
checklist, severity definitions, finding format, the library → component-spec mapping,
|
||||
and how to mark items resolved.
|
||||
|
||||
## Layout
|
||||
|
||||
```
|
||||
code-reviews/
|
||||
├── README.md # this file — process overview + pending findings
|
||||
├── REVIEW-PROCESS.md # how to perform a review and track findings
|
||||
├── regen-readme.py # regenerates this README from the findings files
|
||||
├── _template/findings.md # copy-this template for a library review
|
||||
└── <Library>/findings.md # one folder per ZB.MOM.WW.* shared library
|
||||
```
|
||||
|
||||
## Summary
|
||||
|
||||
0 of 6 libraries reviewed. 0 pending findings across all libraries.
|
||||
|
||||
| Severity | Open findings |
|
||||
|----------|---------------|
|
||||
| Critical | 0 |
|
||||
| High | 0 |
|
||||
| Medium | 0 |
|
||||
| Low | 0 |
|
||||
| **Total** | **0** |
|
||||
|
||||
## Library Status
|
||||
|
||||
| Library | Last reviewed | Commit | Open (C/H/M/L) | Open | Total |
|
||||
|---------|---------------|--------|----------------|------|-------|
|
||||
| [Audit](Audit/findings.md) | — | — | 0/0/0/0 | 0 | 0 |
|
||||
| [Auth](Auth/findings.md) | — | — | 0/0/0/0 | 0 | 0 |
|
||||
| [Configuration](Configuration/findings.md) | — | — | 0/0/0/0 | 0 | 0 |
|
||||
| [Health](Health/findings.md) | — | — | 0/0/0/0 | 0 | 0 |
|
||||
| [Telemetry](Telemetry/findings.md) | — | — | 0/0/0/0 | 0 | 0 |
|
||||
| [Theme](Theme/findings.md) | — | — | 0/0/0/0 | 0 | 0 |
|
||||
|
||||
## Pending Findings
|
||||
|
||||
Every `Open` / `In Progress` finding across all libraries, highest severity first.
|
||||
Resolved findings drop off this list but remain recorded in their library's
|
||||
`findings.md` (see [REVIEW-PROCESS.md](REVIEW-PROCESS.md) §4–§5). Full detail —
|
||||
description, location, recommendation — lives in the library's `findings.md`.
|
||||
|
||||
### Critical (0)
|
||||
|
||||
_None open._
|
||||
|
||||
### High (0)
|
||||
|
||||
_None open._
|
||||
|
||||
### Medium (0)
|
||||
|
||||
_None open._
|
||||
|
||||
### Low (0)
|
||||
|
||||
_None open._
|
||||
@@ -0,0 +1,152 @@
|
||||
# Code Review Process
|
||||
|
||||
This document describes how to perform a comprehensive, per-library code review of the
|
||||
shared libraries hosted in **scadaproj** and how to track findings to resolution.
|
||||
|
||||
scadaproj is an umbrella workspace, but it hosts first-party source: the six
|
||||
`ZB.MOM.WW.*` shared libraries (the realized output of the
|
||||
[component normalizations](../components/README.md)). Those libraries are the code that
|
||||
gets reviewed here. A **library** is one self-contained solution at the repo root (its own
|
||||
`.slnx`, with `src/` and `tests/`) — e.g. `ZB.MOM.WW.Auth/`. Each library has its own
|
||||
folder under `code-reviews/` containing a single `findings.md`.
|
||||
|
||||
The review unit is the **library**, not the individual NuGet package. A library that ships
|
||||
several packages (Auth → 4, Health → 3, Telemetry → 2) is reviewed as a whole; findings
|
||||
point at the specific package/file in their **Location**.
|
||||
|
||||
### The six libraries and their design context
|
||||
|
||||
Each library is reviewed against its normalized component spec. The library folder name is
|
||||
the `ZB.MOM.WW.` prefix stripped; note the spec directory is **not** always the same word.
|
||||
|
||||
| Library folder | Library root | Packages | Component spec |
|
||||
|----------------|--------------|----------|----------------|
|
||||
| `Auth` | `ZB.MOM.WW.Auth/` | `Abstractions`, `Ldap`, `ApiKeys`, `AspNetCore` | [`components/auth/`](../components/auth/) |
|
||||
| `Theme` | `ZB.MOM.WW.Theme/` | single RCL | [`components/ui-theme/`](../components/ui-theme/) |
|
||||
| `Health` | `ZB.MOM.WW.Health/` | `Health`, `.Akka`, `.EntityFrameworkCore` | [`components/health/`](../components/health/) |
|
||||
| `Telemetry` | `ZB.MOM.WW.Telemetry/` | `Telemetry`, `.Serilog` | [`components/observability/`](../components/observability/) |
|
||||
| `Configuration` | `ZB.MOM.WW.Configuration/` | single | [`components/configuration/`](../components/configuration/) |
|
||||
| `Audit` | `ZB.MOM.WW.Audit/` | single | [`components/audit/`](../components/audit/) |
|
||||
|
||||
## 1. Before you start
|
||||
|
||||
1. Pick the library to review. Its review folder is `code-reviews/<Library>/` where
|
||||
`<Library>` is the table above (the `ZB.MOM.WW.` prefix stripped).
|
||||
2. Identify the design context for the library:
|
||||
- Its component spec: `components/<component>/spec/SPEC.md`.
|
||||
- Its proposed shared contract: `components/<component>/shared-contract/ZB.MOM.WW.<Library>.md`.
|
||||
- The library's own `README.md` and the relevant row in the **Component normalization**
|
||||
table in the root [`CLAUDE.md`](../CLAUDE.md).
|
||||
- The adoption backlog `components/<component>/GAPS.md` (what is deliberately deferred).
|
||||
3. Record the exact commit being reviewed: `git rev-parse --short HEAD`. Every review is a
|
||||
snapshot — a finding only means something relative to a known commit.
|
||||
4. Build and test the library so findings are grounded in a green (or known-red) baseline:
|
||||
`cd ZB.MOM.WW.<Library> && dotnet test`.
|
||||
5. Open `code-reviews/<Library>/findings.md` and fill in the header table (reviewer, date,
|
||||
commit SHA).
|
||||
|
||||
## 2. Review checklist
|
||||
|
||||
Work through **every** category below for the library. A comprehensive review means the
|
||||
checklist is completed even where it produces no findings — record "No issues found" for a
|
||||
category rather than leaving it ambiguous.
|
||||
|
||||
These libraries are reusable packages consumed by three separate apps (OtOpcUa,
|
||||
MxAccessGateway, ScadaBridge), so the bar is **API stability and minimal blast radius**:
|
||||
a defect here can break every consumer at once.
|
||||
|
||||
1. **Correctness & logic bugs** — off-by-one, null handling, incorrect conditionals,
|
||||
misuse of BCL/framework APIs, broken edge cases.
|
||||
2. **Public API surface & compatibility** — the public surface is intentional and minimal;
|
||||
naming follows .NET conventions; nullable reference annotations are correct; `sealed` /
|
||||
`abstract` / `virtual` choices are deliberate; no internal types leak through public
|
||||
signatures; evolution is additive-only with semver discipline (libraries are at `0.1.0`).
|
||||
3. **Concurrency & thread safety** — these libraries run inside concurrent hosts; options
|
||||
and value types are immutable, singletons/statics are thread-safe, no shared mutable
|
||||
state, correct `async`/`await`, no sync-over-async.
|
||||
4. **Error handling & resilience** — exceptions thrown to consumers are appropriate and
|
||||
documented; argument validation / guard clauses (`ArgumentNullException` etc.); no
|
||||
swallowed exceptions; fail-fast vs. graceful degradation is deliberate.
|
||||
5. **Security & secret handling** — Auth peppered-HMAC API keys and LDAP-injection surface;
|
||||
redaction seams (`ILogRedactor` in Telemetry, `IAuditRedactor` in Audit); input
|
||||
validation; no secrets or PII in messages, logs, or exception text.
|
||||
6. **Performance & resource management** — `IDisposable` correctness and disposal,
|
||||
allocations on hot paths, buffering/back-pressure, no unnecessary work in DI
|
||||
registration, async all the way down.
|
||||
7. **Spec & shared-contract adherence** — does the code match the library's
|
||||
`components/<component>/spec/SPEC.md` and `shared-contract/ZB.MOM.WW.<Library>.md`?
|
||||
Flag both code that drifts from the normalized contract **and** spec/contract docs that
|
||||
are now stale relative to the code.
|
||||
8. **Packaging, dependencies & project layout** — minimal dependency footprint (e.g. Audit's
|
||||
only non-BCL dependency is `Microsoft.Extensions.DependencyInjection.Abstractions`);
|
||||
correct target framework (.NET 10); package id/version metadata; `dotnet pack` produces
|
||||
the expected nupkg set; central versions in `Directory.Packages.props`; no public
|
||||
dependency leakage; Abstractions-vs-implementation split; Options pattern owned by the
|
||||
library; `AddZb*` DI registration ergonomics.
|
||||
9. **Testing coverage** — are the library's behaviours covered by tests in its `tests/`
|
||||
project? Note untested critical paths, missing edge-case tests, and whether the public
|
||||
contract (not just internals) is exercised.
|
||||
10. **Documentation & XML docs** — XML doc accuracy and presence on the **public** API
|
||||
(these are libraries — public docs matter), library `README.md` correctness, misleading
|
||||
or stale comments, undocumented non-obvious behaviour.
|
||||
|
||||
## 3. Recording findings
|
||||
|
||||
Add one entry per finding to the `## Findings` section of the library's `findings.md`,
|
||||
using the entry format in [`_template/findings.md`](_template/findings.md).
|
||||
|
||||
- **Finding ID** — `<Library>-NNN`, numbered sequentially within the library and never
|
||||
reused (e.g. `Auth-001`, `Telemetry-003`). IDs are permanent even after resolution.
|
||||
- **Severity:**
|
||||
- **Critical** — security breach, data loss, or a defect that crashes/corrupts or
|
||||
deadlocks every consuming app (e.g. a broken public contract, secret leakage).
|
||||
- **High** — incorrect behaviour with significant impact across consumers, or a
|
||||
public-API / back-compat break, with no safe workaround.
|
||||
- **Medium** — incorrect or risky behaviour with limited impact or a workaround.
|
||||
- **Low** — minor issues, style, maintainability, documentation.
|
||||
- **Category** — one of the 10 checklist categories above.
|
||||
- **Location** — `file:line` (clickable), or a list of locations.
|
||||
- **Description** — what is wrong and why it matters.
|
||||
- **Recommendation** — concrete suggested fix.
|
||||
|
||||
After recording findings, update the library header table (status, open-finding count) and
|
||||
refresh the base README (step 5).
|
||||
|
||||
## 4. Marking an item resolved
|
||||
|
||||
Findings are **never deleted** — they are an audit trail. To close one, change its
|
||||
**Status** and complete the **Resolution** field:
|
||||
|
||||
- `Open` — newly recorded, not yet addressed.
|
||||
- `In Progress` — a fix is actively being worked on.
|
||||
- `Resolved` — fixed. The Resolution field must state the fixing commit SHA, the date, and
|
||||
a one-line description of the fix.
|
||||
- `Won't Fix` — intentionally not fixed. The Resolution field must justify why.
|
||||
- `Deferred` — valid but postponed. The Resolution field must say what it is waiting on
|
||||
(e.g. a tracked issue, a `GAPS.md` item, or a later milestone).
|
||||
|
||||
`Resolved`, `Won't Fix`, and `Deferred` findings are all considered **closed** and drop off
|
||||
the base README's pending list. `Open` and `In Progress` are **pending**.
|
||||
|
||||
## 5. Updating the base README
|
||||
|
||||
`code-reviews/README.md` holds the single cross-library view (process overview, the Pending
|
||||
Findings tables, and the Library Status table). It is **generated** from the per-library
|
||||
`findings.md` files — do not edit it by hand.
|
||||
|
||||
After any review or status change, regenerate it:
|
||||
|
||||
```
|
||||
python3 code-reviews/regen-readme.py
|
||||
```
|
||||
|
||||
`regen-readme.py --check` exits non-zero if `README.md` is stale, for use in CI.
|
||||
|
||||
The per-library `findings.md` files are the source of truth; `README.md` is the aggregated
|
||||
index and must always agree with them — which the script guarantees.
|
||||
|
||||
## 6. Re-reviewing a library
|
||||
|
||||
Re-reviews append to the same `findings.md`. Update the header to the new commit and date,
|
||||
continue the finding numbering from the last used ID, and leave prior findings (including
|
||||
closed ones) in place as history.
|
||||
@@ -0,0 +1,36 @@
|
||||
# Code Review — Telemetry
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Library | `ZB.MOM.WW.Telemetry/` |
|
||||
| Packages | `ZB.MOM.WW.Telemetry`, `ZB.MOM.WW.Telemetry.Serilog` |
|
||||
| Component spec | `components/observability/spec/SPEC.md` |
|
||||
| Shared contract | `components/observability/shared-contract/ZB.MOM.WW.Telemetry.md` |
|
||||
| Status | Not yet reviewed |
|
||||
| Last reviewed | — |
|
||||
| Reviewer | — |
|
||||
| Commit reviewed | — |
|
||||
| Open findings | 0 |
|
||||
|
||||
## Summary
|
||||
|
||||
_Not yet reviewed._
|
||||
|
||||
## Checklist coverage
|
||||
|
||||
| # | Category | Examined | Notes |
|
||||
|---|----------|----------|-------|
|
||||
| 1 | Correctness & logic bugs | ☐ | |
|
||||
| 2 | Public API surface & compatibility | ☐ | |
|
||||
| 3 | Concurrency & thread safety | ☐ | |
|
||||
| 4 | Error handling & resilience | ☐ | |
|
||||
| 5 | Security & secret handling | ☐ | |
|
||||
| 6 | Performance & resource management | ☐ | |
|
||||
| 7 | Spec & shared-contract adherence | ☐ | |
|
||||
| 8 | Packaging, dependencies & project layout | ☐ | |
|
||||
| 9 | Testing coverage | ☐ | |
|
||||
| 10 | Documentation & XML docs | ☐ | |
|
||||
|
||||
## Findings
|
||||
|
||||
_No findings recorded yet._
|
||||
@@ -0,0 +1,36 @@
|
||||
# Code Review — Theme
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Library | `ZB.MOM.WW.Theme/` |
|
||||
| Packages | `ZB.MOM.WW.Theme` (Razor Class Library) |
|
||||
| Component spec | `components/ui-theme/spec/SPEC.md` |
|
||||
| Shared contract | `components/ui-theme/shared-contract/ZB.MOM.WW.Theme.md` |
|
||||
| Status | Not yet reviewed |
|
||||
| Last reviewed | — |
|
||||
| Reviewer | — |
|
||||
| Commit reviewed | — |
|
||||
| Open findings | 0 |
|
||||
|
||||
## Summary
|
||||
|
||||
_Not yet reviewed._
|
||||
|
||||
## Checklist coverage
|
||||
|
||||
| # | Category | Examined | Notes |
|
||||
|---|----------|----------|-------|
|
||||
| 1 | Correctness & logic bugs | ☐ | |
|
||||
| 2 | Public API surface & compatibility | ☐ | |
|
||||
| 3 | Concurrency & thread safety | ☐ | |
|
||||
| 4 | Error handling & resilience | ☐ | |
|
||||
| 5 | Security & secret handling | ☐ | |
|
||||
| 6 | Performance & resource management | ☐ | |
|
||||
| 7 | Spec & shared-contract adherence | ☐ | |
|
||||
| 8 | Packaging, dependencies & project layout | ☐ | |
|
||||
| 9 | Testing coverage | ☐ | |
|
||||
| 10 | Documentation & XML docs | ☐ | |
|
||||
|
||||
## Findings
|
||||
|
||||
_No findings recorded yet._
|
||||
@@ -0,0 +1,71 @@
|
||||
# Code Review — <Library>
|
||||
|
||||
<!--
|
||||
Template for a shared-library review. Copy the structure below into
|
||||
code-reviews/<Library>/findings.md and fill it in.
|
||||
<Library> is the ZB.MOM.WW.* library with the prefix stripped (Auth, Theme,
|
||||
Health, Telemetry, Configuration, Audit). See ../REVIEW-PROCESS.md for the
|
||||
full process and the library → component-spec mapping.
|
||||
-->
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Library | `ZB.MOM.WW.<Library>/` |
|
||||
| Packages | `<package ids, e.g. ZB.MOM.WW.<Library>>` |
|
||||
| Component spec | `components/<component>/spec/SPEC.md` |
|
||||
| Shared contract | `components/<component>/shared-contract/ZB.MOM.WW.<Library>.md` |
|
||||
| Status | Not yet reviewed \| In progress \| Reviewed |
|
||||
| Last reviewed | YYYY-MM-DD |
|
||||
| Reviewer | <name> |
|
||||
| Commit reviewed | `<short SHA>` |
|
||||
| Open findings | 0 |
|
||||
|
||||
## Summary
|
||||
|
||||
One short paragraph: overall health of the library, themes across findings, and anything
|
||||
notable that is not a finding (e.g. test count, dependency footprint, adoption status).
|
||||
|
||||
## Checklist coverage
|
||||
|
||||
Confirm every category was examined. Record "No issues found" where applicable.
|
||||
|
||||
| # | Category | Examined | Notes |
|
||||
|---|----------|----------|-------|
|
||||
| 1 | Correctness & logic bugs | ☐ | |
|
||||
| 2 | Public API surface & compatibility | ☐ | |
|
||||
| 3 | Concurrency & thread safety | ☐ | |
|
||||
| 4 | Error handling & resilience | ☐ | |
|
||||
| 5 | Security & secret handling | ☐ | |
|
||||
| 6 | Performance & resource management | ☐ | |
|
||||
| 7 | Spec & shared-contract adherence | ☐ | |
|
||||
| 8 | Packaging, dependencies & project layout | ☐ | |
|
||||
| 9 | Testing coverage | ☐ | |
|
||||
| 10 | Documentation & XML docs | ☐ | |
|
||||
|
||||
## Findings
|
||||
|
||||
<!-- One entry per finding. Copy the block below. Never delete a finding; close it
|
||||
by changing Status and completing Resolution. -->
|
||||
|
||||
### <Library>-001 — <Short title>
|
||||
|
||||
| | |
|
||||
|--|--|
|
||||
| Severity | Critical \| High \| Medium \| Low |
|
||||
| Category | <one of the 10 checklist categories> |
|
||||
| Status | Open \| In Progress \| Resolved \| Won't Fix \| Deferred |
|
||||
| Location | `ZB.MOM.WW.<Library>/src/<Package>/<File>.cs:<line>` |
|
||||
|
||||
**Description**
|
||||
|
||||
What is wrong and why it matters.
|
||||
|
||||
**Recommendation**
|
||||
|
||||
Concrete suggested fix.
|
||||
|
||||
**Resolution**
|
||||
|
||||
_Unresolved._
|
||||
<!-- When closed: fixing commit `<SHA>`, date YYYY-MM-DD, one-line description.
|
||||
For Won't Fix / Deferred, justify the decision here. -->
|
||||
Executable
+198
@@ -0,0 +1,198 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Regenerate code-reviews/README.md from the per-library findings.md files.
|
||||
|
||||
The findings files are the source of truth; README.md is a generated index.
|
||||
Run this after recording, resolving, or re-triaging a finding so the aggregated
|
||||
tables stay in sync (see REVIEW-PROCESS.md section 5).
|
||||
|
||||
Usage:
|
||||
python3 regen-readme.py # rewrite README.md
|
||||
python3 regen-readme.py --check # exit 1 if README.md is stale (for CI)
|
||||
|
||||
Works from any directory — paths are resolved relative to this script.
|
||||
"""
|
||||
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
|
||||
BASE = os.path.dirname(os.path.abspath(__file__))
|
||||
SEVERITY_ORDER = {"Critical": 0, "High": 1, "Medium": 2, "Low": 3}
|
||||
PENDING_STATUSES = {"Open", "In Progress"}
|
||||
NOT_REVIEWED = "—" # rendered for libraries with no recorded review date/commit yet
|
||||
|
||||
|
||||
def discover_libraries():
|
||||
"""Library folders are every subdirectory of code-reviews/ holding a findings.md,
|
||||
excluding the _template folder. Returned sorted for a stable README order."""
|
||||
libraries = []
|
||||
for name in sorted(os.listdir(BASE)):
|
||||
if name.startswith(("_", ".")):
|
||||
continue
|
||||
if os.path.isfile(os.path.join(BASE, name, "findings.md")):
|
||||
libraries.append(name)
|
||||
return libraries
|
||||
|
||||
|
||||
def parse_header(library, text):
|
||||
"""Extract (last_reviewed, commit) from the library's header table.
|
||||
Returns None for either field when it is absent or still templated, so the
|
||||
README can render a 'not yet reviewed' dash instead of a fake baseline."""
|
||||
last = re.search(r"\|\s*Last reviewed\s*\|\s*([0-9]{4}-[0-9]{2}-[0-9]{2})", text)
|
||||
commit = re.search(r"\|\s*Commit reviewed\s*\|\s*`([^`<]+)`", text)
|
||||
return (
|
||||
last.group(1) if last else None,
|
||||
commit.group(1) if commit else None,
|
||||
)
|
||||
|
||||
|
||||
def parse_findings(library):
|
||||
"""Parse one library's findings.md into ((last_reviewed, commit), [(library, id, severity, title, status), ...])."""
|
||||
text = open(os.path.join(BASE, library, "findings.md")).read()
|
||||
header = parse_header(library, text)
|
||||
findings = []
|
||||
for block in re.split(r"^### ", text, flags=re.M)[1:]:
|
||||
head = block.splitlines()[0].strip()
|
||||
m = re.match(r"([A-Za-z][A-Za-z0-9]*-\d+)\b(.*)", head)
|
||||
if not m:
|
||||
raise SystemExit(f"{library}/findings.md: unparseable finding heading: {head!r}")
|
||||
fid = m.group(1).strip()
|
||||
title = m.group(2).strip().lstrip("—–-").strip().replace("|", "\\|")
|
||||
sev = re.search(r"\|\s*Severity\s*\|\s*([A-Za-z]+)", block)
|
||||
status = re.search(r"\|\s*Status\s*\|\s*([A-Za-z' ]+?)\s*\|", block)
|
||||
if not sev or not status:
|
||||
raise SystemExit(f"{library}/findings.md: {fid} is missing a Severity or Status field")
|
||||
findings.append((library, fid, sev.group(1), title, status.group(1).strip()))
|
||||
return header, findings
|
||||
|
||||
|
||||
def finding_number(finding):
|
||||
return int(re.search(r"-(\d+)$", finding[1]).group(1))
|
||||
|
||||
|
||||
def build_readme(libraries, per_library):
|
||||
pending = sorted(
|
||||
(f for fs in per_library.values() for f in fs[1] if f[4] in PENDING_STATUSES),
|
||||
key=lambda f: (SEVERITY_ORDER.get(f[2], 9), f[0], finding_number(f)),
|
||||
)
|
||||
reviewed = sum(1 for lib in libraries if per_library[lib][0][0] is not None)
|
||||
|
||||
def severity_total(sev):
|
||||
return sum(1 for f in pending if f[2] == sev)
|
||||
|
||||
def open_count(library, sev):
|
||||
return sum(1 for f in per_library[library][1]
|
||||
if f[2] == sev and f[4] in PENDING_STATUSES)
|
||||
|
||||
lines = []
|
||||
add = lines.append
|
||||
|
||||
add("# Code Reviews")
|
||||
add("")
|
||||
add("Comprehensive, per-library code reviews of the `ZB.MOM.WW.*` shared libraries hosted")
|
||||
add("in this repo. Each library (one self-contained `.slnx` at the repo root) has its own")
|
||||
add("folder containing a `findings.md`. This README is the aggregated index — the single")
|
||||
add("place to see all outstanding work.")
|
||||
add("")
|
||||
add("> Generated by `regen-readme.py` from the per-library `findings.md` files. Do not")
|
||||
add("> edit by hand — edit the findings files and re-run the script.")
|
||||
add("")
|
||||
add("## How it works")
|
||||
add("")
|
||||
add("- Reviews are performed one library at a time against a fixed checklist.")
|
||||
add("- Each library is reviewed against its normalized component spec under `components/`.")
|
||||
add("- Every finding is recorded in the library's `findings.md` with a severity and status.")
|
||||
add("- Findings are **never deleted** — they are closed by changing their status, keeping")
|
||||
add(" a full audit trail.")
|
||||
add("- This README aggregates every **pending** finding (`Open` / `In Progress`) across all")
|
||||
add(" libraries.")
|
||||
add("")
|
||||
add("See **[REVIEW-PROCESS.md](REVIEW-PROCESS.md)** for the full procedure: the review")
|
||||
add("checklist, severity definitions, finding format, the library → component-spec mapping,")
|
||||
add("and how to mark items resolved.")
|
||||
add("")
|
||||
add("## Layout")
|
||||
add("")
|
||||
add("```")
|
||||
add("code-reviews/")
|
||||
add("├── README.md # this file — process overview + pending findings")
|
||||
add("├── REVIEW-PROCESS.md # how to perform a review and track findings")
|
||||
add("├── regen-readme.py # regenerates this README from the findings files")
|
||||
add("├── _template/findings.md # copy-this template for a library review")
|
||||
add("└── <Library>/findings.md # one folder per ZB.MOM.WW.* shared library")
|
||||
add("```")
|
||||
add("")
|
||||
add("## Summary")
|
||||
add("")
|
||||
add(f"{reviewed} of {len(libraries)} libraries reviewed. "
|
||||
f"{len(pending)} pending finding{'s' if len(pending) != 1 else ''} across all libraries.")
|
||||
add("")
|
||||
add("| Severity | Open findings |")
|
||||
add("|----------|---------------|")
|
||||
for sev in ("Critical", "High", "Medium", "Low"):
|
||||
add(f"| {sev} | {severity_total(sev)} |")
|
||||
add(f"| **Total** | **{len(pending)}** |")
|
||||
add("")
|
||||
add("## Library Status")
|
||||
add("")
|
||||
add("| Library | Last reviewed | Commit | Open (C/H/M/L) | Open | Total |")
|
||||
add("|---------|---------------|--------|----------------|------|-------|")
|
||||
for library in libraries:
|
||||
counts = [open_count(library, s) for s in ("Critical", "High", "Medium", "Low")]
|
||||
last_reviewed, commit = per_library[library][0]
|
||||
last_cell = last_reviewed if last_reviewed else NOT_REVIEWED
|
||||
commit_cell = f"`{commit}`" if commit else NOT_REVIEWED
|
||||
add(f"| [{library}]({library}/findings.md) | {last_cell} | {commit_cell} "
|
||||
f"| {counts[0]}/{counts[1]}/{counts[2]}/{counts[3]} "
|
||||
f"| {sum(counts)} | {len(per_library[library][1])} |")
|
||||
add("")
|
||||
add("## Pending Findings")
|
||||
add("")
|
||||
add("Every `Open` / `In Progress` finding across all libraries, highest severity first.")
|
||||
add("Resolved findings drop off this list but remain recorded in their library's")
|
||||
add("`findings.md` (see [REVIEW-PROCESS.md](REVIEW-PROCESS.md) §4–§5). Full detail —")
|
||||
add("description, location, recommendation — lives in the library's `findings.md`.")
|
||||
add("")
|
||||
for sev in ("Critical", "High", "Medium", "Low"):
|
||||
rows = [f for f in pending if f[2] == sev]
|
||||
add(f"### {sev} ({len(rows)})")
|
||||
add("")
|
||||
if not rows:
|
||||
add("_None open._")
|
||||
add("")
|
||||
continue
|
||||
add("| ID | Library | Title |")
|
||||
add("|----|---------|-------|")
|
||||
for library, fid, _, title, _ in rows:
|
||||
add(f"| {fid} | [{library}]({library}/findings.md) | {title} |")
|
||||
add("")
|
||||
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def main():
|
||||
check = "--check" in sys.argv[1:]
|
||||
libraries = discover_libraries()
|
||||
per_library = {m: parse_findings(m) for m in libraries}
|
||||
content = build_readme(libraries, per_library)
|
||||
|
||||
readme_path = os.path.join(BASE, "README.md")
|
||||
pending = sum(1 for fs in per_library.values()
|
||||
for f in fs[1] if f[4] in PENDING_STATUSES)
|
||||
total = sum(len(fs[1]) for fs in per_library.values())
|
||||
|
||||
if check:
|
||||
current = open(readme_path).read() if os.path.exists(readme_path) else ""
|
||||
if current != content:
|
||||
print("README.md is stale — run: python3 code-reviews/regen-readme.py")
|
||||
sys.exit(1)
|
||||
print(f"README.md is up to date ({pending} pending / {total} total).")
|
||||
return
|
||||
|
||||
open(readme_path, "w").write(content)
|
||||
print(f"README.md regenerated — {pending} pending, {total} total findings "
|
||||
f"across {len(libraries)} libraries.")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
Reference in New Issue
Block a user