feat: implement issue #258 — Compliance: copilot-instructions-missing-local-dev-commands - #272
feat: implement issue #258 — Compliance: copilot-instructions-missing-local-dev-commands#272don-petry wants to merge 4 commits into
Conversation
…-local-dev-commands
|
Warning Review limit reached
More reviews will be available in 59 minutes and 46 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive .github/copilot-instructions.md file outlining the Broodly project's tech stack, structure, local development commands, environment variables, testing framework, and repo-specific overrides. It also modifies .gitignore by adding another duplicate .dev-lead/ entry. The feedback suggests cleaning up these redundant entries in .gitignore.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 384878e121
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pnpm format:check # Prettier check (CI-safe, no writes) | ||
|
|
||
| # --- Code generation --- | ||
| cd apps/api && go generate ./... # Regenerate gqlgen resolvers and sqlc queries |
There was a problem hiding this comment.
The command block already changes into apps/api on line 67, so running line 80 in the same shell (cd apps/api && go generate ./...) tries to enter apps/api/apps/api, which does not exist and prevents code generation from running. This breaks the documented workflow for anyone executing the commands top-to-bottom.
Useful? React with 👍 / 👎.
|
|
||
| ## Required Environment Variables | ||
|
|
||
| See `apps/mobile/.env.example` and `apps/api/.env.example` for full lists. Key variables: |
There was a problem hiding this comment.
This points contributors to apps/mobile/.env.example and apps/api/.env.example, but those files are not present in the repository, so the instructions send users to missing paths when they try to discover required variables. That makes initial local setup and troubleshooting materially harder.
Useful? React with 👍 / 👎.
| | Server State | TanStack React Query (persistent cache) | | ||
| | UI State | Zustand | | ||
| | GraphQL Client | urql or Apollo Client | | ||
| | Backend | Go 1.24 (chi + gqlgen) | |
There was a problem hiding this comment.
This advertises the backend as Go 1.24, but apps/api/go.mod enforces toolchain go1.26.2 for required security fixes, so developers following this table may provision the wrong version and hit avoidable setup failures (especially where automatic toolchain download is restricted). The instructions should reflect the actual required toolchain.
Useful? React with 👍 / 👎.
| pnpm format:check # Prettier check (CI-safe, no writes) | ||
|
|
||
| # --- Code generation --- | ||
| cd apps/api && go generate ./... # Regenerate gqlgen resolvers and sqlc queries |
There was a problem hiding this comment.
The comment says this regenerates gqlgen and sqlc outputs, but the repository currently has no //go:generate directives under apps/api, so go generate ./... is a no-op and does not perform the documented regeneration step. This makes the local dev guidance incorrect for codegen maintenance.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a .github/copilot-instructions.md to satisfy the org copilot-instructions-missing-local-dev-commands compliance check (#258), giving Copilot a project overview, tech stack, structure, local dev commands, env vars, testing notes, and repo-specific overrides. Also appends another .dev-lead/ entry to .gitignore.
Changes:
- New
.github/copilot-instructions.mdwith the required## Local Dev Commandssection and surrounding context. - Adds a duplicate
.dev-lead/line to.gitignore.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
.github/copilot-instructions.md |
New always-on Copilot instructions covering stack, structure, local dev commands, env vars, testing, and overrides. |
.gitignore |
Appends another .dev-lead/ entry on top of existing duplicates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ``` | ||
| broodly/ | ||
| ├── apps/ | ||
| │ ├── mobile/ # Expo app — screens in app/, features in src/features/ | ||
| │ └── api/ # Go GraphQL API — domain/, service/, repository/ under internal/ | ||
| ├── packages/ | ||
| │ ├── ui/ # Shared Gluestack-based component library | ||
| │ ├── graphql-types/ # Generated TypeScript types (never hand-write these) | ||
| │ ├── domain-types/ # Shared constants and domain types | ||
| │ ├── config/ # Shared configuration | ||
| │ └── test-utils/ # Shared testing utilities | ||
| ├── infra/terraform/ # GCP infrastructure as code | ||
| ├── tests/ # Integration and e2e tests | ||
| └── docs/ # ADRs, architecture docs, runbooks | ||
| ``` | ||
|
|
||
| Key conventions: | ||
| - Shared UI components → `packages/ui/src/`, exported from `packages/ui/src/index.ts` | ||
| - Feature components → `apps/mobile/src/features/<feature>/components/` | ||
| - Screen files → `apps/mobile/app/` (Expo Router file-based routing) | ||
| - Package boundaries are hard: `packages/ui` must not import from `apps/mobile` |
| | Server State | TanStack React Query (persistent cache) | | ||
| | UI State | Zustand | | ||
| | GraphQL Client | urql or Apollo Client | | ||
| | Backend | Go 1.24 (chi + gqlgen) | |
|
|
||
| ## Required Environment Variables | ||
|
|
||
| See `apps/mobile/.env.example` and `apps/api/.env.example` for full lists. Key variables: |
| pnpm format:check # Prettier check (CI-safe, no writes) | ||
|
|
||
| # --- Code generation --- | ||
| cd apps/api && go generate ./... # Regenerate gqlgen resolvers and sqlc queries |
|
|
||
| - **Mobile / packages:** Jest with `@testing-library/react-native`. Tests co-located as `ComponentName/ComponentName.test.tsx`. Run with `pnpm --filter mobile test`. | ||
| - **API:** Standard `go test ./...`. Integration tests require a running PostgreSQL instance; see `docs/runbooks/` for local DB setup. | ||
| - **E2E:** Maestro flows in `tests/e2e/` (run in CI only). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8d4d55a5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cd apps/api | ||
| go run cmd/server/main.go # Start API server | ||
| go test ./... # Run all Go tests | ||
| go vet ./... # Go static analysis |
There was a problem hiding this comment.
This API section only tells developers to run go vet, but .github/workflows/ci.yml also runs golangci/golangci-lint-action in apps/api pinned to v2.11.4. For changes that trigger golangci-lint rules beyond vet, contributors can pass every documented local API check and still fail CI, so add the matching local golangci-lint run ./... command/version here.
Useful? React with 👍 / 👎.
| go vet ./... # Go static analysis | ||
|
|
||
| # --- Monorepo-wide --- | ||
| pnpm test # Run all JS/TS tests across all packages |
There was a problem hiding this comment.
When this fenced block is followed top-to-bottom, line 67 leaves the shell in apps/api, but these pnpm commands are root package scripts from /workspace/broodly/package.json; apps/api has no package manifest or JS/TS scripts. Since pnpm run runs scripts from a package manifest (https://pnpm.io/cli/run), users copying the documented flow can run the monorepo checks from the wrong directory instead of exercising the workspace; add a cd ../.. or use pnpm -C ../.. ... before this section.
Useful? React with 👍 / 👎.
|
|
||
| - **Mobile / packages:** Jest with `@testing-library/react-native`. Tests co-located as `ComponentName/ComponentName.test.tsx`. Run with `pnpm --filter mobile test`. | ||
| - **API:** Standard `go test ./...`. Integration tests require a running PostgreSQL instance; see `docs/runbooks/` for local DB setup. | ||
| - **E2E:** Maestro flows in `tests/e2e/` (run in CI only). |
There was a problem hiding this comment.
This says Maestro E2E flows under tests/e2e/ run in CI, but I checked .github/workflows/ci.yml and it only has the TypeScript and Go jobs, while tests/e2e/ contains only .gitkeep and there is no Maestro command in the repo. Contributors relying on these instructions will assume E2E coverage is enforced when it currently is not, so either add the CI job/flows or soften this to planned guidance.
Useful? React with 👍 / 👎.
| # --- API server (Go) --- | ||
| cd apps/api | ||
| go run cmd/server/main.go # Start API server | ||
| go test ./... # Run all Go tests |
There was a problem hiding this comment.
The documented API test command omits the flags used by CI: .github/workflows/ci.yml runs go test ./... -race -coverprofile=coverage.out from apps/api. For code that passes normal tests but introduces a data race, contributors following these local instructions will miss the failure until CI, so document the CI-equivalent command alongside the quick go test command.
Useful? React with 👍 / 👎.
| ## Testing Framework | ||
|
|
||
| - **Mobile / packages:** Jest with `@testing-library/react-native`. Tests co-located as `ComponentName/ComponentName.test.tsx`. Run with `pnpm --filter mobile test`. | ||
| - **API:** Standard `go test ./...`. Integration tests require a running PostgreSQL instance; see `docs/runbooks/` for local DB setup. |
There was a problem hiding this comment.
This tells developers to see docs/runbooks/ for local PostgreSQL setup before running integration tests, but that directory only contains .gitkeep and no setup instructions. Anyone with DB-dependent tests is left without the referenced prerequisite steps, so add the runbook or remove this pointer until it exists.
Useful? React with 👍 / 👎.
Dev-Lead — fix-bot-comment (applied)Changes committed and pushed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05a37f1448
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| # --- Code generation (run from apps/api) --- | ||
| cd apps/api | ||
| go run github.com/99designs/gqlgen generate # Regenerate gqlgen resolvers |
There was a problem hiding this comment.
When contributors follow this new codegen step from apps/api, this go run is resolved in the current module because it has no @version suffix (checked go help run), but apps/api/go.mod/go.sum do not require github.com/99designs/gqlgen (rg 99designs apps/api/go.mod apps/api/go.sum returns no matches). In a normal module-aware Go setup this fails with “no required module provides package” instead of regenerating resolvers, so either add a tracked tool dependency or use a versioned command such as .../gqlgen@<version>.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,2 @@ | |||
| EXPO_PUBLIC_API_URL=http://localhost:8080/graphql | |||
| FIREBASE_PROJECT_ID=your-firebase-project-id | |||
There was a problem hiding this comment.
For mobile code that reads the Firebase project ID in the Expo JavaScript bundle, copying this example leaves the value unavailable: Expo only inlines .env variables with the EXPO_PUBLIC_ prefix into client code (see Expo environment variable docs: https://docs.expo.dev/guides/environment-variables/). Since the new instructions mark this variable as used by mobile + api, the mobile example should expose a public, non-secret name such as EXPO_PUBLIC_FIREBASE_PROJECT_ID rather than FIREBASE_PROJECT_ID.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df0e7859e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # --- Code generation (run from apps/api) --- | ||
| cd apps/api | ||
| go run github.com/99designs/gqlgen generate # Regenerate gqlgen resolvers | ||
| sqlc generate # Regenerate sqlc queries |
There was a problem hiding this comment.
This newly documented sqlc generate step is not runnable in the current API tree: apps/api/sqlc.yaml points at internal/repository/queries/ and migrations/, but I checked the tracked files under apps/api and neither input directory exists. Contributors who follow the local-dev commands will hit a sqlc input-path error instead of regenerating code, so either add the expected inputs/placeholders or mark this as a planned command until the repository contains them.
Useful? React with 👍 / 👎.
|
|
Closing due to merge conflict that cannot be auto-rebased. Re-implementing from fresh main via dev-lead. |
Pull request was closed
Closes #258
Implemented by dev-lead agent. Please review.