Skip to content

docs: add CONTRIBUTING.md - #87

Merged
vernonstinebaker merged 2 commits into
mainfrom
docs/contributing
Oct 3, 2026
Merged

vernonstinebaker merged 2 commits into
mainfrom
docs/contributing

Conversation

@vernonstinebaker

Copy link
Copy Markdown
Contributor

Adds the contribution flow: build prerequisites, commit gates (zig build test, zig fmt, ui build), the 4-target CI matrix including the e2e suite, the regression-test mandate, and a lifecycle-hygiene rule for instance process management (distilled from #86). Complements the existing AGENTS.md. Docs-only.

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

Checked the guide against AGENTS.md, README.md, build.zig, ui/package.json, the actual CI inputs and #86. The four-target matrix and native-Linux E2E description match the workflow. The lifecycle requirement is appropriate for the existing supervisor/CLI boundary, and the guide adds a contribution flow rather than duplicating the architecture documentation. It merges cleanly with current main.

One non-blocking improvement: it would be great to use bash tests/test_backend.sh as the backend gate (or list both commands from it). CI and README currently run both zig build test and zig build test-integration with -Dembed-ui=false -Dbuild-ui=false; the new checklist only mentions unit tests. Using the existing script keeps the instructions consistent, includes the real-process HTTP integration suite, and avoids an unnecessary UI build for backend-only checks.

No blocking findings in this documentation-only PR.

@vernonstinebaker

Copy link
Copy Markdown
Contributor Author

Addressed in d76930e — step 4 now uses bash tests/test_backend.sh, the script CI actually invokes (ci.yml:30 test_command), and names both commands it wraps (zig build test + zig build test-integration, both with -Dembed-ui=false -Dbuild-ui=false).

That also fixes a second inaccuracy in the old line: it said zig build test --summary all with no -D flags and no integration suite, while README:211-212 and CI both use the flags and run both steps.

Left the "4-target CI matrix" wording as-is — verified it's correct here: targets_json in ci.yml has exactly those 4 entries and all 4 are required checks (unlike nullwatch, where the shared default is 3).

Docs-only, +20/-0 on one new file. CI running; approval retained.

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

Rechecked d76930e against tests/test_backend.sh and .github/workflows/ci.yml. The contributor gate now matches both backend unit and integration checks, including the UI build flags. The four-target matrix remains accurate. The earlier nit is resolved; approval stands.

@vernonstinebaker
vernonstinebaker merged commit 825b0cd into main Oct 3, 2026
4 checks passed
@vernonstinebaker
vernonstinebaker deleted the docs/contributing branch October 3, 2026 16:39
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.

2 participants