Contributing to BetterVoting — process notes (for next time)¶
A record of how the first code contribution went, so the workflow (and its gotchas) isn't re-derived from scratch. Written after PR #1419 — "Clean up the ballot-data export (JSON v2 + CSV Raw/Official)".
Outcome (recorded 2026-08-07) — the PR was SPLIT, and half of it shipped. A maintainer (jacksonloper) opened #1428 four days later, extracting this PR's CSV bug fixes onto a branch named
masiarek/csv-escaping-fix, explicitly crediting the author and keeping Adam's authorship on the commit (309d84b0, in BV'smainhistory). It merged 2026-07-16, and added a third bug fix of its own (the Download menu unmounting mid-fetch). The JSON v2 format and the Raw/Official CSV split were held back — "the rest needs some contemplation" — and #1419 was closed on 2026-07-15. So the tracking issue #1420 stays open and BV still ships the v1 export, documented in the BV JSON export reference.Follow-up (2026-08-07): the format half is back as PR #1492, reshaped to be purely additive —
+391/−0, the existing "Download JSON" untouched and byte-identical, the compact shape offered as a second menu item. Deletions are the thing a maintainer has to think about; a diff with none of them asks a much smaller question. SamebuildElectionExport, now verified across 197 distinct real exports as well as the unit suite.The lesson for next time: land bug fixes and format/product changes as separate PRs. No technical objection to the v2 format was ever posted — the only review comment on #1419 was CodeRabbit flagging one stale test assertion. What stalled it is that a bundled PR forces an all-or-nothing decision, and changing an existing public download is a judgement call a maintainer can't make on a Tuesday, while fixing a corrupt CSV is obvious. Bundled, the obvious part waits on the deliberate part; split, the obvious part merges in a week. A maintainer doing the split by hand is a friendly outcome, not a rebuff — but it is work you can do yourself, and doing it yourself keeps the deliberate half alive as an open PR instead of a closed one.
Running it locally (dev server vs Docker, and the AirPlay / Keycloak /
crypto.randomUUID/ shared-rebuild gotchas) → running_bettervoting_locally.md.
The setup facts (easy to forget)¶
- Upstream repo:
Equal-Vote/bettervoting(was formerly "star-server"). - My fork:
github.com/masiarek/star-server— it kept the old name from when I forked it years ago, but it's still a fork ofEqual-Vote/bettervoting, so it works fine. - I do NOT have direct push to
Equal-Vote/bettervoting.git push originreturns403 denied to masiarek. Contributions go fork → PR. (They may grant me direct contributor access later, but I've never asked, and there's still a required approval by another person either way.) - Local clone:
/Volumes/T7/Voting/BetterVoting/BV/bettervoting, remoteorigin = Equal-Vote/bettervoting. I add aforkremote pointing at my fork:git remote add fork https://github.com/masiarek/star-server.git - Git identity wasn't set in this clone — had to
git config user.email/user.nameonce.
The workflow we followed (export-cleanup example)¶
- Diagnosed the problem. The "Download JSON" export was literally
JSON.stringify({Election, Ballots, Results})of the raw in-memory objects (packages/frontend/src/components/Election/Results/BallotDataExport.tsx), leaking the tabulator's internal shape (O(n²) pairwise maps, mixed casing, inconsistent timestamps). - Fixed it in the right layer. The export is a view, so the transform went in
packages/shared/src/utils/exportFormat.ts(buildElectionExport) — no change to the tabulation engine, the results endpoint, or what the UI renders. Wired it intodownloadJson. - Wrote a unit test.
packages/backend/src/test/exportFormat.test.ts(Jest). - Verified on real data. Ran the actual transform against saved exports in
06_Other/_demo_dropbox/— 69% of legacy size on a 51-candidate election, 60% on a small one, all defects gone. (This is what proved it before ever touching CI.) - Branch + commit.
git checkout -b feature/clean-json-export git add <the 3 files only> git commit # descriptive message: what changed and why - Push to my fork.
git push -u fork feature/clean-json-export - Open the PR against upstream.
→ PR #1419.
gh pr create --repo Equal-Vote/bettervoting \ --base main --head masiarek:feature/clean-json-export \ --title "..." # choose the pull_request_template, fill Description / Screenshots / Related Issues - (Optional) File a tracking issue and add
fixes #Nto the PR's Related Issues. Caveat: issue creation may be restricted in this repo ("Issue creation is restricted"). If so, skip it — the PR is self-contained. - Approval. Another maintainer reviews/approves before merge. That's the gate — not my push access.
Gotchas hit (and fixes)¶
- Stale
packages/backend/build/directory shadowssrcand creates duplicate Jest haste-map mocks; it also made babel choke on TS in the new test.rm -rf packages/backend/buildbeforenpm test -w @equal-vote/star-vote-backend. Worth a separate PR to gitignorebuild/. <placeholders>in pasted commands — angle brackets are shell redirection; they error with "Read-only file system" / "no such file". Always substitute the real value.--body-fileneeds the real path (quoted if it has spaces).
Why Docker was NOT needed (the surprise)¶
I installed Docker assuming I'd need it — I didn't, for this kind of change. Here's the split:
Docker (docker compose up) spins up the full runtime stack — app + PostgreSQL + Keycloak + nginx + Playwright. You need it when you want to run the actual application or run end-to-end (Playwright) tests, because those need a live database, auth server, and browser.
What we did needed none of that:
- The change was a pure function (
buildElectionExport) — no DB, no auth, no network, no running server. - Unit tests are Jest — plain Node, no services.
npm test -w @equal-vote/star-vote-backend. - Type/build checks are
tsc— plain Node. - Verification ran the transpiled function against saved JSON exports on disk.
So the rule of thumb:
| Task | Docker needed? |
|---|---|
Build / typecheck (tsc, npm run build) |
No |
| Lint | No |
| Unit tests (Jest, tabulators, pure functions) | No |
| Run the app locally end-to-end | Yes (needs Postgres + Keycloak) — or provide those yourself |
| E2E / Playwright tests | Yes (the compose stack includes them) |
| Anything touching the DB layer / migrations against a real Postgres | Yes (or a local Postgres) |
Bottom line: for a self-contained frontend/shared change covered by unit tests, Docker is overkill. Keep it installed for the day you touch the backend runtime, auth, migrations, or need to run the E2E suite.
Reusable checklist¶
- [ ] Change in the smallest correct layer (view vs engine vs DB).
- [ ] Unit test added (Jest — no Docker).
- [ ] Verified on real data where possible.
- [ ]
git checkout -b feature/..., stage only the intended files, descriptive commit. - [ ]
git push -u fork <branch>(fork =masiarek/star-server). - [ ]
gh pr create --repo Equal-Vote/bettervoting --base main --head masiarek:<branch>. - [ ] Fill the PR template; link an issue if creation is allowed.
- [ ] Wait for another maintainer's approval.