Skip to content

docs: add CONTRIBUTING.md - #52

Open
Waynting wants to merge 1 commit into
aloth:mainfrom
Waynting:docs/contributing
Open

docs: add CONTRIBUTING.md#52
Waynting wants to merge 1 commit into
aloth:mainfrom
Waynting:docs/contributing

Conversation

@Waynting

@Waynting Waynting commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

First of the two PRs for #49. The architecture note follows separately, as
you asked.

Root CONTRIBUTING.md, with the README one-liner kept and pointing at it.

What is in it

The four setup facts from the issue — npm ci, the exact Node floor, the
e2e*.sh glob, how to run the e2e suite — plus three things that came out of
writing it:

The e2e suites are not all runnable by a contributor. test/e2e.sh takes
OLCLI_E2E_PROJECT_NAME and works for anyone, but test/e2e-ignore.sh and
test/e2e-issue7.sh have PROJECT_ID="697fca16..." hardcoded near the top.
Documented as a caveat rather than changed — making them configurable is a
code change and did not belong in a docs PR. Happy to do it as a follow-up if
you want it.

Where a new test should go. The unit suite needs no account and no
network, and most of olcli is deliberately written so it can be tested that
way — the pure modules, plus the local-HTTP-server pattern in
test/client.test.ts. Saying so is what stops the next contributor reaching
for e2e by default and then finding it will not run in CI.

Lint warnings as a budget. npm run lint exits zero with a pile of
no-explicit-any warnings. Without a note, a contributor either thinks the
repo is broken or decides to fix them all inside an unrelated PR.

Your two repository behaviours are in "What to expect from CI", together with
the refs/pull/<n>/merge point — all three share the shape of "this looks
like a broken pull request and is not", so they read better together than
scattered.

The release section explains the tag trigger and the Homebrew bump, but leads
with the part that affects a contributor: it is why the CHANGELOG entry is
asked for, since Unreleased becomes the release notes.

One thing to decide

The intro points at docs/ARCHITECTURE.md, which arrives in the second PR. I
left it as a plain code span rather than a link so nothing is broken in the
meantime, but it does assume the follow-up lands. Say the word and I will drop
the line, or you can merge them in order.

Nothing here is executable, so CI has nothing new to check. Every claim was
checked against the source rather than from memory: ci.yml, publish.yml,
package.json, and the three e2e scripts.

Closes half of aloth#49. The Contributing section in the README was one line, and
none of what actually trips someone up on a fresh clone was written down
anywhere a contributor reads first.

Covers: npm ci rather than npm install and why CI enforces it; the Node floor
being an exact patch version; the three checks CI runs and the fourth that
verifies entry points were emitted; what the lint warning count means; which
tests need a real Overleaf account and why test/e2e*.sh sits outside the
npm test glob; how to run the e2e suites, including that two of them have a
maintainer's project ID hardcoded; writing a CHANGELOG entry, since nothing
generates one; and the three repository behaviours that look like a broken
pull request but are not - the first-contribution approval gate, CI testing
refs/pull/<n>/merge, and when the Update branch button appears.

The README one-liner stays and now points at the file.
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.

1 participant