196 lines
5.2 KiB
Markdown
196 lines
5.2 KiB
Markdown
# Contributing to OrderOps
|
|
|
|
This repo is used to simulate a small product team environment. Treat every change like a real ticket and a real pull request.
|
|
|
|
## Purpose
|
|
|
|
Students are expected to use their coding harness to:
|
|
|
|
- inspect the relevant code paths before editing
|
|
- compare their change against this document before opening a PR
|
|
- run the right checks locally
|
|
- produce a PR summary that explains behavior, testing, and risk
|
|
|
|
Use this file as the review rubric.
|
|
|
|
## Local development
|
|
|
|
```bash
|
|
bun install
|
|
bun run seed
|
|
bun run seed:user
|
|
# tab 1
|
|
bun run dev:api
|
|
# tab 2
|
|
bun run dev:web
|
|
```
|
|
|
|
Common checks:
|
|
|
|
```bash
|
|
bun run check
|
|
bun run test
|
|
bun run build:web
|
|
```
|
|
|
|
## Using your coding harness
|
|
|
|
Ask your harness to do these things explicitly:
|
|
|
|
1. trace the feature or bug through the repo before changing code
|
|
2. compare your branch against `CONTRIBUTING.md`
|
|
3. identify missing tests or missing manual verification
|
|
4. check whether your change follows existing file and naming patterns
|
|
5. draft your PR summary and testing notes
|
|
|
|
Good prompt pattern:
|
|
|
|
- `Explain code path for this ticket and list files likely affected.`
|
|
- `Review my changes against CONTRIBUTING.md and list gaps before PR.`
|
|
- `Generate PR summary, testing notes, and follow-ups from my diff.`
|
|
|
|
## OrderOps team conventions
|
|
|
|
These are repo-specific rules. Follow them even when they differ from general habits.
|
|
|
|
### File organization
|
|
|
|
Do:
|
|
|
|
- keep files grouped by feature and purpose
|
|
- use action-based filenames like `update-order-item.ts` or `order-routes.ts`
|
|
- keep shared contracts in `packages/shared`
|
|
- put database access in repository files
|
|
- put business rules in domain or use-case files
|
|
- let page-level UI files own data loading
|
|
- keep presentational components focused and small
|
|
|
|
Do not:
|
|
|
|
- add broad `utils.ts`, `helpers.ts`, or `misc.ts` dump files
|
|
- hide unrelated business rules in one generic helper
|
|
- move unrelated files just to match personal preference
|
|
- create new abstractions before there is a second real use
|
|
|
|
### Backend conventions
|
|
|
|
Do:
|
|
|
|
- keep route handlers thin
|
|
- validate at the API boundary
|
|
- keep business rules authoritative on the backend
|
|
- name functions after business actions
|
|
- keep audit and status logic readable in business terms
|
|
|
|
Do not:
|
|
|
|
- put complex business rules directly in route handlers
|
|
- spread status checks across random layers when one clear place exists
|
|
- rely on frontend validation alone
|
|
|
|
### Frontend conventions
|
|
|
|
Do:
|
|
|
|
- keep page components responsible for loading and mutation wiring
|
|
- keep child components focused on rendering and local interaction
|
|
- use clear labels and copy that support and ops staff would understand
|
|
- prefer explicit prop names over short clever names
|
|
|
|
Do not:
|
|
|
|
- bury fetch logic deep in presentational components
|
|
- add styling abstractions when plain Tailwind utilities are clearer
|
|
- rename concepts away from business language used elsewhere in repo
|
|
|
|
### Naming and searchability
|
|
|
|
Do:
|
|
|
|
- prefer direct names over clever names
|
|
- keep one clear responsibility per file when practical
|
|
- make it easy to find logic with repo search
|
|
|
|
Do not:
|
|
|
|
- introduce abbreviations that make search harder
|
|
- split one small behavior across many tiny files without a good reason
|
|
|
|
### Comments and copy
|
|
|
|
Do:
|
|
|
|
- use comments for business context, constraints, or non-obvious reasoning
|
|
- write test names in business language
|
|
- write PR summaries in terms of behavior impact
|
|
|
|
Do not:
|
|
|
|
- add comments that only restate obvious code
|
|
- describe implementation only without explaining user or business impact
|
|
|
|
## Testing expectations
|
|
|
|
Do:
|
|
|
|
- add or update tests when behavior changes
|
|
- prefer API or domain tests for business logic
|
|
- keep tests focused on public behavior
|
|
- run the cheapest relevant checks first
|
|
|
|
Current repo checks:
|
|
|
|
- `bun run check`
|
|
- `bun run test`
|
|
- `bun run build:web`
|
|
|
|
Manual checks are still expected when UI or workflow behavior changes.
|
|
|
|
## PR readiness checklist
|
|
|
|
Before opening a PR, verify all of these:
|
|
|
|
- [ ] change is scoped to one ticket or one small reviewable concern
|
|
- [ ] acceptance criteria for the ticket have been checked
|
|
- [ ] code follows file organization and layering rules in this doc
|
|
- [ ] no unrelated refactor is mixed in
|
|
- [ ] debug code, temporary logs, and dead code are removed
|
|
- [ ] relevant automated checks were run locally
|
|
- [ ] changed behavior was manually tested if needed
|
|
- [ ] PR description explains behavior impact, not only implementation
|
|
- [ ] known risks, limitations, or follow-ups are called out
|
|
|
|
## Reviewer expectations
|
|
|
|
Reviewers will usually look for:
|
|
|
|
- smallest correct change
|
|
- behavior preserved outside requested scope
|
|
- business rules in the right layer
|
|
- tests covering the important path
|
|
- filenames and structure that match repo conventions
|
|
- clear PR summary and testing notes
|
|
|
|
Expect review comments about missing tests, misplaced business logic, unclear abstractions, and unnecessary scope.
|
|
|
|
## Future CI requirements
|
|
|
|
These are expected to become required CI gates:
|
|
|
|
- typecheck: `bun run check`
|
|
- tests: `bun run test`
|
|
- web production build: `bun run build:web`
|
|
- lint/format: planned follow-up once lint tooling is added
|
|
|
|
## Pull request format
|
|
|
|
Use the PR template in `.github/pull_request_template.md`.
|
|
|
|
Every PR should include:
|
|
|
|
- short summary
|
|
- linked ticket or assignment
|
|
- what changed
|
|
- how it was tested
|
|
- risks or follow-ups
|