Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
125 lines (79 loc) · 11.5 KB

CONTRIBUTING.md

File metadata and controls

125 lines (79 loc) · 11.5 KB
Raw

Contributing to quantcli/common

This repo holds the contract and shared conventions every quantcli *-export-cli agrees to. A change here is a change every CLI must follow. That means PRs land slowly and deliberately. If you want to move fast, work in the per-service repo (crono-export-cli, liftoff-export-cli, withings-export-cli); only promote to common once the change has stabilized in at least one of them.

Before you open a PR

  • Open an issue or comment on an existing one first if the change touches CONTRACT.md or affects all three exporters. The contract is a coordination point — discussion before code saves rework.
  • Trivial fixes (typos, dead links, README polish) can skip the discussion step and go straight to a PR.
  • Don't add dependencies casually. The Go side is standard library first; markdown tooling stays in CI, not in the repo.

Branch conventions

Branch off main. Use a short prefix:

  • docs/ — README, CONTRACT, this file, anything documentation-only.
  • chore/ — repo plumbing, CI tweaks, version bumps.
  • feat/ — new shared code or new section of CONTRACT.md.
  • fix/ — bug fixes in shared code.

Examples: docs/clarify-since-keyword, chore/bump-go-1.23, feat/contract-csv-quoting.

Branches are short-lived. Rebase on main rather than merging main back in. One branch, one PR, one logical change.

Commit style

  • Imperative subject, ≤ 72 chars: docs: clarify --since keyword behavior.
  • Optional body explains why, wrapped at ~72 chars.
  • One concern per commit. If a PR has unrelated cleanups, split them.
  • Reference the issue or contract section when relevant: Refs CONTRACT.md §3.

Pull requests

  • Title mirrors the lead commit.
  • Body answers three questions: What changed? Why? What does it imply for the per-service CLIs? That third question is the one PR authors miss most often, and it's the one reviewers care about most.
  • Mark the PR as draft until CI is green.
  • Squash-merge by default. Use a merge commit only if the individual commits are independently meaningful (rare).
  • The Lead Go Engineer is the merge gate for this repo. Expect one round of review on most PRs.

Proposing a contract change

CONTRACT.md is the user-facing surface of every export CLI. Changes here ripple. The expected flow:

  1. Open an issue describing the proposed change. Include: the current behavior, the proposed behavior, why the change is worth the coordination cost, and a list of which *-export-cli repos are affected.
  2. Land the implementation in at least one exporter first, behind whatever flag or path lets it ship without breaking the contract. This is how we avoid contract changes that look fine in prose and fall apart on contact with a real upstream API.
  3. Open a PR against common that updates CONTRACT.md and the Status table. The PR body lists the follow-up issues filed against each affected *-export-cli repo so reviewers can see the rollout plan.
  4. Update compat tests in the same PR (see below).
  5. Don't merge until every affected exporter either has the change shipped or has a tracked, owner-assigned follow-up issue. A merged contract change with no rollout plan creates silent drift.

User-facing flag changes follow semver in each CLI: a removed or renamed flag is a major bump.

Adding a new export-cli to the family

The bar for joining the family is conformance to CONTRACT.md, not feature parity. A new CLI is welcome once it satisfies all of:

  1. Repo and binary naming match §1. The repo lives at github.com/quantcli/{service}-export-cli; the binary is {service}-export.
  2. Date flags behave per §3. --since and --until parse local-calendar dates with the keyword, absolute, and relative forms; relative durations snap to local midnight; --until is inclusive of the named day.
  3. Output formats match §4. --format markdown|json|csv, markdown default, data on stdout, errors on stderr, exit 0 on empty result.
  4. Auth has the surface in §5. An auth status subcommand exists and exits non-zero when the CLI cannot make an authenticated request. Headless env vars are documented in prime where the upstream allows them.
  5. prime subcommand is present with the section structure in §6 and fits on one terminal screen.
  6. Compat tests pass — see below.

Once all six are met, open a PR against this repo that:

  • Adds the new repo to the README's "Repos that follow this contract" list.
  • Adds a column to the Status table in CONTRACT.md (the only contract change permitted as part of onboarding a new CLI; everything else is a separate proposal).
  • Links to the new CLI's first tagged release.

Keep the per-CLI PR (the one that adds the CLI to the table) trivial and reviewable. Behavioral conformance is verified by the compat tests, not by re-reading the new CLI's source from this repo.

Compat tests

The contract is only as honest as the test that proves three CLIs behave the same. The harness lives in compat/ as its own Go module (github.com/quantcli/common/compat); each exporter imports it and runs the relevant bundles against its own built binary in CI.

Rules:

  • Anyone changing CONTRACT.md is also expected to update or add tests under compat/ that exercise the new behavior against every *-export-cli.
  • The harness is deliberately black-box: it shells out to the binary and asserts on stdout, stderr, and exit code only. It must not import a CLI's internal packages.
  • One subpackage per contract section. The naming convention is compat/<section> where <section> is the CONTRACT.md section being attested — currently compat/dates (§2–§3) and compat/formats (§4); compat/auth (§5) and compat/prime (§6) are expected to follow. Each subpackage exposes a single entry point — RunContract(t, runner) — that exporters call from one build-tagged _test.go file.
  • Cobra-based exporters whose contract surface lives on subcommands set compat.Runner.Subcommands; section bundles dispatch per-subcommand under a subcommand=NAME/... subtree. Flat CLIs leave the field empty and the bundle runs against the root binary. Nested paths (e.g. liftoff-export workouts stats) are passed as a single space-separated entry — the dispatcher splits on whitespace so each segment is its own argv entry.
  • A PR that changes the contract without touching compat/ is incomplete. Either update the tests in the same PR or open a follow-up issue and link it from the PR body before merging — the Lead Go Engineer holds the line on this.
  • Compat tests run in CI on every PR and on main. A failing compat test on main means at least one shipped CLI no longer matches the contract, and that's a release-blocker incident, not a flake.
  • The Status table in CONTRACT.md distinguishes machine-attested rows (covered by compat/) from human-attested rows (still verified by reviewer judgment). Promoting a row from human to machine attestation is itself a worthwhile PR.

Citing the contract from compat code

If you write a comment in compat/ that quotes the contract, use the form CONTRACT §N: "exact text". Every such quote is auto-verified at PR time by TestCONTRACTCitationsAreReal in compat/citations_test.go — the test walks the compat tree and fails CI if the quoted fragment is not a substring of section N of CONTRACT.md. The check uses double-quoted fragments only; backtick-wrapped code references (e.g. `--format`) are ignored.

Two ways to make a failing citation green:

  • Drop the quote, or restate the property without claiming it comes from the contract.
  • Update CONTRACT.md so §N actually contains the text, then re-run go test ./... from compat/.

The check exists because two consecutive PRs landed CONTRACT §5: quotes that referenced text §5 didn't contain. That class of error should die in CI now, not in a post-merge incident comment.

Bar for a new exporter: the exporter's CI must build its binary and run dates.RunContract against it green. The formats.RunContract bundle ships, but its CSV subtest assumes an exporter that implements all three §4 codecs (markdown, json, csv) — until the framework gains a Runner.SupportedFormats affordance, exporters whose CSV writer is incomplete cannot adopt the bundle as a hard gate. Wire it in as exporter parity catches up. See compat/README.md for the one-file integration pattern.

Supply-chain and security

Every PR — here and in each *-export-cli — is gated on the security workflow defined in .github/workflows/security.yml. It runs three checks:

  • govulncheck — Go vulnerability database scan against the module's full dependency closure (skipped when there is no go.mod).
  • osv-scanner — OSV transitive vulnerability scan over the working tree.
  • License policy — every direct + transitive Go dependency must resolve to one of the permissive SPDX ids listed in SECURITY.md. GPL-family, SSPL, BSL, and other non-permissive licenses are blocking.

If a PR fails the license-policy check, do not unblock it by adding a replace directive or vendoring a fork to relabel the license. Either drop the dependency, switch to a permissively licensed equivalent, or open an issue requesting a policy exception with the rationale.

To propose a security policy change (allowlist, severity bar, or workflow tooling), open an issue here first — these decisions ripple to every export-cli.

Where the workflow lives, and when to centralize it

The security workflow is copied verbatim into every repo that runs it (quantcli/common plus each *-export-cli). quantcli/common is the source of truth; the # security.yml v1 — source of truth: quantcli/common marker at the top of each copy is the version key. When you change the workflow in common, propagate the change (and bump the version) to every repo that runs it. A future drift-check job will key off that marker.

Self-contained is the right shape while the org has ≤5 repos running this workflow. The tradeoff is deliberate: each repo's CI is hermetic, no org-level reusable-workflow setting to wrangle, and new contributors can read the entire pipeline in one file. The cost is drift — fixes in common don't propagate automatically, and the version marker plus this propagation rule is the mitigation.

Switchover trigger: when a 6th repo onboards the workflow (i.e. >5 repos), evaluate moving to a workflow_call reusable workflow hosted in quantcli/common and consumed by each export-cli. At that point the copy-and-propagate cost exceeds the centralization cost. Revisit sooner if GitHub starts letting non-admins read the reusable-workflow org setting (today that's an admin-only surface, which would otherwise hide CI configuration from contributors). See Lead Go review on PR #5 for the original rationale.

For reporting a vulnerability in a shipped CLI or in common, see SECURITY.md. Do not open a public issue with exploit details.

License and sign-off

This repo is MIT-licensed (see LICENSE). By contributing you agree your changes are under the same license. We don't require a CLA. Sign-off (Signed-off-by: in commit messages) is encouraged but not required.

Questions

Open an issue. Tag the Lead Go Engineer if it's blocking you. See TRIAGE.md for the time-to-first-touch SLA and how reports are labeled and routed across the four quantcli/* repos.