Skip to content

Commit 76ec78e

Browse files
jdaltonjdalton
committed
docs(contributing): add setup + workflow + pre-PR checklist
Covers the setup (Node 22+, pnpm, git submodule init), repo layout, the two-vitest-config split (standard vs isolated), the 100% coverage rule, the parallel-sessions git rules from CLAUDE.md (no git stash, no git add -A, worktree for branches), conventional commits (no AI attribution), and a copy-paste pre-PR checklist. Junior-dev level: every command has a one-line purpose, hazards section names what has tripped contributors before.
1 parent 0dad065 commit 76ec78e

2 files changed

Lines changed: 317 additions & 0 deletions

File tree

‎docs/contributing.md‎

Lines changed: 311 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,311 @@
1+
# Contributing
2+
3+
What you need to know before opening a PR against
4+
`@socketregistry/packageurl-js`. Dev setup, the test/coverage/lint
5+
workflow, the parallel-session git rules, and the pre-PR checklist.
6+
7+
## Who this is for
8+
9+
First-time contributors and returning ones who want to refresh on
10+
the commands and conventions. Reading order: **Setup → Making a
11+
change → Running checks → Opening the PR**.
12+
13+
## Setup
14+
15+
Requirements:
16+
17+
| Tool | Version | Notes |
18+
|---|---|---|
19+
| Node.js | ≥ 22 | Built-in TypeScript stripping (`--experimental-strip-types` on 22, default on 23+). |
20+
| pnpm | ≥ 11.0.0-rc.0 | Install via `corepack enable pnpm` or `npm install -g pnpm`. |
21+
| git | ≥ 2.30 | Submodule support. |
22+
23+
Clone and install:
24+
25+
```bash
26+
git clone https://github.com/SocketDev/socket-packageurl-js.git
27+
cd socket-packageurl-js
28+
pnpm install
29+
```
30+
31+
`pnpm install` also:
32+
33+
- Sets up the `husky` pre-commit hooks.
34+
- Initializes `upstream/meander` submodule lazily on first
35+
`pnpm tour:build`.
36+
37+
`pnpm tour doctor` reports which external tools the build may shell
38+
out to are present vs missing on your machine. All listed tools are
39+
optional; missing ones fall back to safe defaults.
40+
41+
## Repo layout
42+
43+
```
44+
socket-packageurl-js/
45+
├── src/ ← library source (see docs/architecture.md)
46+
├── test/ ← functional tests (vitest)
47+
├── scripts/ ← build/lint/test/release scripts
48+
├── docs/ ← these docs
49+
├── val/ ← Val Town comment backend (tour-specific)
50+
├── upstream/meander/ ← submodule (tour generator)
51+
├── .config/ ← tsconfig + vitest configs
52+
├── .github/workflows/ ← CI (ci.yml, pages.yml, provenance.yml, ...)
53+
├── .claude/ ← Claude Code config (agents, skills, hooks)
54+
├── tour.json ← tour manifest
55+
└── walkthrough/ ← tour build output (gitignored)
56+
```
57+
58+
You almost never edit `upstream/meander/` (that's the submodule
59+
we pin) or `walkthrough/` (build output).
60+
61+
## Making a change
62+
63+
### 1. Branch, worktree, or sibling clone
64+
65+
This repo expects **multiple concurrent Claude Code sessions** may
66+
be working on the same checkout. Plain `git checkout -b` inside the
67+
main clone yanks the working tree out from under any other session.
68+
Use one of:
69+
70+
- **Worktree** — `git worktree add -b my-task ../socket-packageurl-js-my-task main`
71+
- **Sibling clone** — clone the repo elsewhere entirely
72+
- **Same clone** — only if you are sure nobody else has a session
73+
going
74+
75+
See the `PARALLEL CLAUDE SESSIONS` section of `CLAUDE.md` for the
76+
full doctrine.
77+
78+
### 2. Edit source
79+
80+
Per `CLAUDE.md`:
81+
82+
- TypeScript strict mode; type imports must be in separate
83+
`import type` lines.
84+
- No `null` except `__proto__: null` or external-API requirement —
85+
use `undefined`.
86+
- `{ __proto__: null, ...payload }` for config/return/internal
87+
objects.
88+
- No dynamic imports (`await import(...)`).
89+
- No `fetch()` — use `httpJson` / `httpText` / `httpRequest` from
90+
`@socketsecurity/lib/http-request`.
91+
- For file existence: `existsSync` from `node:fs`. For deletion:
92+
`safeDelete` / `safeDeleteSync` from `@socketsecurity/lib/fs` —
93+
never reach directly for `fs.rm` / `fs.unlink` / `fs.rmdir`.
94+
- Default to NO comments in code. Only where the WHY is
95+
non-obvious to a senior engineer.
96+
- Use the `Edit` tool for text changes; never `sed` / `awk`.
97+
98+
### 3. Write tests
99+
100+
Two vitest configs:
101+
102+
| Config | When | File naming |
103+
|---|---|---|
104+
| `.config/vitest.config.mts` | Normal tests. Threads, shared memory. Fast. | `*.test.mts` |
105+
| `.config/vitest.config.isolated.mts` | Tests that mock globals via `vi.doMock`, modify `process.env`, or `process.chdir`. Forks, full isolation. | `*.isolated.test.mts` |
106+
107+
**Test style in this repo:** functional. Tests assert behavior via
108+
the public API — inputs → outputs. Tests never read source files
109+
and assert on their contents.
110+
111+
Example:
112+
113+
```typescript
114+
import { test, expect } from 'vitest'
115+
import { PackageURL } from '../src/package-url.js'
116+
117+
test('parses npm scoped package', () => {
118+
const purl = new PackageURL('pkg:npm/@scope/pkg@1.0.0')
119+
expect(purl.namespace).toBe('@scope')
120+
expect(purl.name).toBe('pkg')
121+
expect(purl.version).toBe('1.0.0')
122+
})
123+
```
124+
125+
If you are adding a new ecosystem or URL parser, put tests under
126+
`test/purl-types/<name>.test.mts` or
127+
`test/url-converter/<name>.test.mts`.
128+
129+
## Running checks
130+
131+
The **canonical pre-PR check** is a single command:
132+
133+
```bash
134+
pnpm check
135+
```
136+
137+
It runs:
138+
139+
1. `pnpm type` — tsgo strict type-check.
140+
2. `pnpm lint` — oxlint across the tree.
141+
3. `pnpm test` — vitest (both configs).
142+
4. Format verification.
143+
144+
Every step runs independently too:
145+
146+
| Command | What |
147+
|---|---|
148+
| `pnpm build` | Compile `src/` → `dist/` (esbuild). `pnpm build --watch` for dev. |
149+
| `pnpm type` | Strict TypeScript check, no emit. |
150+
| `pnpm lint` | oxlint. |
151+
| `pnpm fix` | Auto-fix what's auto-fixable (formatter + lint autofixes). |
152+
| `pnpm test` | Run vitest. `pnpm test:unit path/to/file.test.mts` for a single file. Never use `--` before test paths — runs ALL tests. |
153+
| `pnpm testu` | Update vitest snapshots (review the diff before committing). |
154+
| `pnpm cover` | Coverage. Must stay at 100%. |
155+
| `pnpm format` | Run oxfmt across the tree (writes fixes). |
156+
| `pnpm format --check` | Verify formatting without writing. |
157+
| `pnpm security` | AgentShield + zizmor security scan. |
158+
159+
### Coverage — the 100% rule
160+
161+
`pnpm cover` must report 100% line/branch/function coverage.
162+
163+
If you add a code path, add a test that exercises it. If you
164+
discover a corner case while reading, add a test for it even if the
165+
coverage number already says 100% (percentages can miss edge
166+
cases).
167+
168+
When coverage drops, the failure names the file and the missed
169+
line/branch. Add a test or document an `/* c8 ignore next */` with
170+
a justification comment.
171+
172+
### Snapshot tests
173+
174+
`pnpm testu` regenerates snapshots. Never run this unless the
175+
snapshot drift is intentional — otherwise you are deleting future-
176+
you's safety net. Review the snapshot diff before committing.
177+
178+
## The parallel-session git rules
179+
180+
Forbidden in the primary checkout (the one another session may be
181+
editing):
182+
183+
- `git stash` / `git stash pop` — shared store; another session
184+
can pop yours.
185+
- `git add -A` / `git add .` — sweeps files belonging to other
186+
sessions.
187+
- `git checkout <branch>` / `git switch <branch>` — yanks the
188+
working tree.
189+
- `git reset --hard` against a non-HEAD ref — discards another
190+
session's commits.
191+
192+
Required for branch work:
193+
194+
```bash
195+
git worktree add -b <task-branch> ../<repo>-<task> main
196+
cd ../<repo>-<task>
197+
# edit, commit, push from here
198+
cd -
199+
git worktree remove ../<repo>-<task>
200+
```
201+
202+
Required for staging:
203+
204+
```bash
205+
git add <specific-file> [<file>…]
206+
```
207+
208+
Never `-A` / `.`.
209+
210+
For a quick WIP save: commit on a new branch from inside a
211+
worktree, not a stash.
212+
213+
## Commit messages
214+
215+
[Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/):
216+
217+
```
218+
<type>(<scope>): <description>
219+
220+
<optional body>
221+
222+
<optional footer>
223+
```
224+
225+
Types used in this repo:
226+
227+
- `feat` — new feature
228+
- `fix` — bug fix
229+
- `refactor` — restructure without behavior change
230+
- `docs` — documentation only
231+
- `test` — tests only
232+
- `chore` — tooling, deps, repo config
233+
- `style` — formatting / whitespace
234+
235+
Scopes are free-form but stable: `purl-types/npm`, `url-converter`,
236+
`tour`, `hardening`. One commit = one concern. If the body
237+
explains both a refactor and a fix, they should probably be two
238+
commits.
239+
240+
**Never add AI attribution** ("Co-authored-by: Claude", etc.) to
241+
commit messages. Humans commit; tools assist.
242+
243+
## Opening a PR
244+
245+
After `pnpm check` passes and all commits are conventional:
246+
247+
```bash
248+
gh pr create --title "<conventional title>" --body "…"
249+
```
250+
251+
The PR body should:
252+
253+
- Summarize the change (1–3 bullets).
254+
- Include a test plan as a markdown checklist.
255+
- Link any related issue (`Fixes #123` if applicable).
256+
257+
CI will run:
258+
259+
- `.github/workflows/ci.yml` — full `pnpm check` in a clean env.
260+
- `.github/workflows/pages.yml` — if you touched tour sources,
261+
rebuilds the tour and deploys to GH Pages on merge to main.
262+
- `.github/workflows/valtown.yml` — if you touched `val/`,
263+
deploys the comment backend after merge.
264+
- `.github/workflows/provenance.yml` — on tag, signs + publishes
265+
to npm with attestations.
266+
267+
## Pre-PR checklist
268+
269+
Copy into your PR description and tick off:
270+
271+
- [ ] `pnpm check` passes locally.
272+
- [ ] `pnpm cover` still at 100%.
273+
- [ ] New tests for new behavior (functional style, public API).
274+
- [ ] No `null`, no `fetch()`, no `fs.rm`/`unlink`/`rmdir` direct
275+
calls.
276+
- [ ] Commit messages are Conventional, no AI attribution.
277+
- [ ] Documentation touched if user-facing behavior or API changed.
278+
- [ ] For new ecosystem handlers: entry in `src/purl-types/`,
279+
registered in `src/purl-type.ts`, builder factory in
280+
`src/package-url-builder.ts`, tests.
281+
- [ ] `CLAUDE.md` + `docs/*` updated if conventions shifted.
282+
283+
## Hazards
284+
285+
Things that have caught contributors before:
286+
287+
- **Forgetting to bump `upstream/meander`** when the pin is stale.
288+
`pnpm tour:build --refresh` will re-clone and rebuild.
289+
- **Running `pnpm testu` by accident** — if you only wanted to
290+
run tests, use `pnpm test`. `testu` updates snapshots.
291+
- **Editing `dist/` directly** — it's a build artifact. Changes
292+
here are lost on next build. Edit `src/` and rebuild.
293+
- **`pnpm install` side-effects** — the install step runs `husky`
294+
to set up git hooks. A bare `npm install` in this repo will skip
295+
that and PRs will push with unformatted files. Use pnpm.
296+
- **`.env.local` accidentally committed** — it's gitignored; don't
297+
`git add -A`.
298+
- **`minimumReleaseAge` exclusions** — NEVER add packages to
299+
`minimumReleaseAgeExclude` in CI. Locally, ask before adding.
300+
The age threshold is a security control.
301+
302+
## Further reading
303+
304+
- [`CLAUDE.md`](../CLAUDE.md) — project-wide conventions, error
305+
message doctrine, safe-deletion rule, parallel-sessions rules.
306+
- [`docs/architecture.md`](./architecture.md) — how `src/` fits
307+
together.
308+
- [`docs/tour.md`](./tour.md) — the build pipeline that ships
309+
these docs as HTML.
310+
- [`docs/release.md`](./release.md) — what happens when we tag.
311+
- [`package.json`](../package.json) — every script spelled out.

‎tour.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,12 @@
179179
"source": "docs/builders.md",
180180
"summary": "The PurlBuilder fluent API — construct a PackageURL step by step with per-field setters and per-ecosystem factories."
181181
},
182+
{
183+
"filename": "contributing",
184+
"title": "Contributing",
185+
"source": "docs/contributing.md",
186+
"summary": "Dev setup, test layout, the parallel-sessions git workflow, and the full pre-PR checklist."
187+
},
182188
{
183189
"filename": "converters",
184190
"title": "Converters",

0 commit comments

Comments
 (0)