Feat/onboarding guide#2647
Conversation
illegalcall
left a comment
illegalcall
left a comment
There was a problem hiding this comment.
Reviewed against the current main branch and issue #2648. I left the actionable documentation findings inline. One additional history-level issue: commits 032fbf3 and 85ce973 are titled ‘nits’, which does not follow the repository’s conventional-commit guidance. Local verification completed with npm run lint, git diff --check, and Prettier passing.
| Read these first: | ||
|
|
||
| 1. **[AGENTS.md](AGENTS.md)** - repo layout, daemon/API boundaries, coding conventions, and hard rules. Required reading before writing code. | ||
| 2. **[Development Guide](docs/development.md)** - prerequisites, build steps, running tests, and troubleshooting. |
There was a problem hiding this comment.
[P3] This description promises troubleshooting, but docs/development.md has no troubleshooting section. Please add common setup/build failure guidance or stop advertising troubleshooting here.
| - Stay updated on the latest developments and roadmap | ||
| - **Issues:** Use GitHub issues for bug reports and feature requests (see | ||
| templates). | ||
| - **Security issues:** Report privately per our [Security Policy](SECURITY.md). |
There was a problem hiding this comment.
[P1] SECURITY.md does not exist in the merged target, so this private-reporting path leads to a 404. Please add the policy or link to a valid private security contact before merging.
|
|
||
| | Tool | Minimum version | Notes | | ||
| | ---------- | --------------- | ------------------------------------------------------------------- | | ||
| | Go | 1.25 | `go version` to check; install via [go.dev](https://go.dev/dl/) | |
There was a problem hiding this comment.
[P2] These minimums are lower than the authoritative manifests: backend/go.mod requires Go 1.25.7, and the locked Vite 8.0.16 requires Node ^20.19.0 or >=22.12.0. Please update the table to the actual supported floors.
| | Go | 1.25 | `go version` to check; install via [go.dev](https://go.dev/dl/) | | ||
| | Node.js | 20 | `node --version`; install via [nodejs.org](https://nodejs.org/) | | ||
| | npm | 10 | Ships with Node.js | | ||
| | Nix (opt.) | - | `nix develop` drops you into a shell with all deps; see `flake.nix` | |
There was a problem hiding this comment.
[P3] This relative link resolves to docs/flake.nix, which does not exist. The repository file is at the root, so the target should be ../flake.nix.
|
|
||
| ```bash | ||
| git clone https://github.com/AgentWrapper/agent-orchestrator.git | ||
| cd agent-orchestrator |
There was a problem hiding this comment.
[P2] A fresh clone still has no root dependencies after these steps, but the later npm run api command needs the root openapi-typescript dev dependency. Please add a root npm ci/npm install step to the clean-clone setup.
|
|
||
| ```bash | ||
| cd backend | ||
| golangci-lint run ./... |
There was a problem hiding this comment.
[P2] golangci-lint is not installed by the prerequisites or flake and this invocation is unpinned. AGENTS.md defines the supported command as npm run lint, which pins v2.12.2; please use that command or document installation and the required version.
| - Unit tests use Vitest and run in a simulated renderer environment. | ||
| - E2E tests use Playwright with a full Electron app. | ||
| - After changing API types, run `npm run api` from root to regenerate | ||
| `src/api/schema.ts`. |
There was a problem hiding this comment.
[P2] Since this says to run from the repository root, src/api/schema.ts is the wrong path. The generated artifact is frontend/src/api/schema.ts, as the later artifact list correctly states.
| @@ -0,0 +1,258 @@ | |||
| # Development Guide | |||
There was a problem hiding this comment.
[P3] Please add this new onboarding guide to docs/README.md (and ideally the root README documentation table) so contributors can discover it from the repository’s documentation indexes.
2 participants
Updated Contribution.md
and Added docs/development.md file which will works as an Onboarding Docs for users who are new to the project
issue: #2648