Skip to content

docs: add CONTRIBUTING.md#371

Open
interacsean wants to merge 3 commits into
mainfrom
docs/update-contributing
Open

docs: add CONTRIBUTING.md#371
interacsean wants to merge 3 commits into
mainfrom
docs/update-contributing

Conversation

@interacsean

Copy link
Copy Markdown
Contributor

Summary

  • Adds CONTRIBUTING.md documenting the end-to-end contributor lifecycle (issue → branch → develop → quality-check → changeset → PR → review → release), sourced from a previously-drafted (stashed) version.
  • Updated the draft to match current repo state:
    • Added the check-licenses CI workflow (introduced in ci: add check-licenses workflow #353) to the CI table and CI/CD workflow reference.
    • Added the now in-repo api-design-review skill (.agents/skills/api-design-review/) alongside the existing Copilot prompt.
    • Removed the /board-issue command reference — that's a personal Claude Code skill some contributors have installed locally, not something checked into this repo, so the doc now describes filing on the project board generically instead.

Test plan

  • pnpm exec oxfmt --check CONTRIBUTING.md passes
  • Team reviews wording/accuracy of the workflow description

Documents the end-to-end contributor workflow (issue -> branch -> develop
-> quality-check -> changeset -> PR -> review -> release), updated to
reflect the current skills/CI setup: the check-licenses workflow and the
in-repo api-design-review skill, and removing a reference to a personal
board-issue skill that isn't checked into this repo.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@interacsean interacsean requested a review from a team as a code owner July 9, 2026 01:34
Comment thread CONTRIBUTING.md

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good, but how do we manage the relationship between this and CLAUDE.md?

Some information looks overlapped

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep good point. We should have CLAUDE.md refer to CONTRIBUTING.md as an include so it is ensured to be read. I think a more human-friendly CONTRIBUTING is useful, as I did not find all the processes clear and intuitive when I came back to work on AppShell a few weeks ago

Comment thread CONTRIBUTING.md Outdated

### Adding or changing a UI component → `add-component` skill

`.agents/skills/add-component/SKILL.md` is the full procedure. The house rules it enforces:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This goes beyond linking the skill and re-documents several mutable rules from add-component (patterns, styling rules, export policy, test expectations). Since SKILL.md is the source of truth, could we trim this to a short summary + link? Otherwise every convention change has to be updated in two places.

Comment thread CONTRIBUTING.md Outdated

## 5. Run quality checks locally

Use the **`quality-check`** skill (`.agents/skills/quality-check/SKILL.md`) — run these in

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same drift concern here: this reproduces the quality-check procedure plus extra workflow details (lefthook, local API review, E2E setup). We already have one mismatch: ci-e2e.yaml includes VITE_TAILOR_AI_GATEWAY_URL, but the env list here does not. I'd prefer linking the skill/workflow instead of maintaining a second copy.

Comment thread CONTRIBUTING.md Outdated

| Workflow | Trigger | What it does |
| ------------------ | ----------------------------------------------------------------------------------------- | ----------------------------------------------------------------- |
| **CI (packages)** | push touching `packages/**`, `examples/**`, `docs/**`, `catalogue/**`, lockfile/manifests | `pnpm install` → `fmt:check` → `turbo run lint test type-check` |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This CI table seems to have already drifted from the workflow files. For example, ci-packages.yaml and ci-e2e.yaml are pull_request workflows today, but this says push. That makes me nervous about documenting trigger mechanics inline here unless we have a generated source.

Comment thread CONTRIBUTING.md Outdated

## Reference

### Skills

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These reference tables are useful, but they also duplicate a lot of file-by-file metadata (roles, triggers, outputs, tooling notes). Could we keep this section to pointers to .agents/skills/, .github/workflows/, and CLAUDE.md/README.md instead? This feels like the highest-maintenance part of the document.

Addresses review feedback: the doc was re-documenting mutable rules from
add-component/quality-check skills and CI trigger mechanics inline, which
had already drifted (the CI table claimed `push` triggers when both
ci-packages.yaml and ci-e2e.yaml are pull_request-triggered, and the e2e
env var list was missing VITE_TAILOR_AI_GATEWAY_URL). Every section that
duplicated a skill/workflow's content now points at it instead.

Also link CLAUDE.md -> CONTRIBUTING.md so agents are guaranteed to read
the contributor workflow doc, per review discussion.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@interacsean

Copy link
Copy Markdown
Contributor Author

Thanks both — re-evaluated based on the common thread across these comments: the doc was re-documenting mutable rules (add-component's house rules, quality-check's procedure, CI trigger mechanics, four reference tables) that live authoritatively elsewhere, and it had already drifted as a result — the CI table said push when ci-packages.yaml/ci-e2e.yaml are actually pull_request-triggered, and the e2e env var list was missing VITE_TAILOR_AI_GATEWAY_URL.

Pushed a trim (7ed1d16) that keeps the narrative/lifecycle framing but replaces every restated rule/table with a pointer to its source of truth (.agents/skills/**, .github/workflows/, etc.) so there's exactly one place to update per convention. Also linked CLAUDE.mdCONTRIBUTING.md per @interacsean's comment, so coding agents are guaranteed to load it.

- Fold the duplicated CLAUDE.md positioning note into the intro so CLAUDE
  is characterized once, not in two back-to-back paragraphs.
- Replace the restated pnpm install/dev + version-pinning block in §1 with
  a pointer to README's Development section (the canonical home for
  commands); keep the repo-layout table, which README lacks.
- Drop CLAUDE.md's redundant "Local Development" block (it already points
  to README for commands just above).
- Fix stale fact: packages/core builds with Vite, not tsup.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants