Skip to content

Document the development workflow in CONTRIBUTING.md - #340

Open
maragubot wants to merge 4 commits into
maragudk:mainfrom
maragubot:garden/document-development-workflow
Open

Document the development workflow in CONTRIBUTING.md#340
maragubot wants to merge 4 commits into
maragudk:mainfrom
maragubot:garden/document-development-workflow

Conversation

@maragubot

Copy link
Copy Markdown
Contributor

What was found

CONTRIBUTING.md explained the social process for contributing — submit a PR
directly for small changes, open an issue first for larger ones, plus the
licensing terms — but never said how to build, test, or lint the project.

The Makefile has test, lint, cover, benchmark, and fuzz targets, and
AGENTS.md documents make test, make lint, and the coverage rule. But
AGENTS.md is addressed to AI assistants, so a human contributor had no
documented way to verify a change before opening a PR. The file has had no
build or test guidance in its entire history.

What was fixed

A ## Development section covering the five make targets, the coverage
expectation, and the Go version floor.

Each claim was checked by running it:

  • make test passes, and every package outside internal/ reports 100.0%.
    The section says "outside internal/" because go tool cover -func reports
    97.5% overall — internal/assert has no tests and is excluded via
    codecov.yml, which the docs did not mention anywhere.
  • .golangci.yml declares version: "2", and golangci-lint v1 refuses to run
    against it, so the section asks for v2 specifically.
  • make fuzz passes -fuzztime 10s per target, matching a timed run.
  • The CI test matrix covers Go 1.18 through 1.26. A go 1.18 directive stops
    newer language features at compile time but silently allows newer standard
    library
    packages, so a green local run can still fail CI. Rather than tell
    contributors to avoid newer APIs, the section points at the convention the
    repo already follows: gomponents_benchmark_test.go and
    html/benchmark_test.go use testing.B.Loop behind //go:build go1.24.

Review notes

Two reviewers checked this by execution over two rounds. Everything they raised
is resolved. Their first pass caught the bare "100% coverage" claim
contradicting what make cover actually shows, the missing golangci-lint v2
requirement, and a tautological over-long line; the second pass caught that an
earlier draft telling contributors to "avoid newer language and library
features" contradicted the repo's own build-tagged benchmarks.

Two findings were left alone as out of scope for a single-issue PR, noted here
in case they are worth separate passes:

  • AGENTS.md now duplicates make test, make lint, and the coverage rule,
    and is the less precise of the two on coverage. Worth deciding whether it
    should defer to CONTRIBUTING.md.
  • internal/examples/app is a separate module with its own go.mod and
    go.work, so neither make test nor CI touches it. A contributor editing
    the example app gets no verification at all.

The contributor guide covered how to submit a PR but never how to verify a
change. Add a Development section with the `make` targets for tests, linting,
coverage, benchmarks, and fuzzing, plus the 100% coverage expectation.
Reviewers pointed out that `make cover` reports 97.5%, not 100%, because
`internal/` is only excluded via `codecov.yml`; that `.golangci.yml` requires
golangci-lint v2; and that CI tests back to Go 1.18, which is the most likely
way a green local run still fails CI.
The repo already uses Go 1.24's `testing.B.Loop` in `gomponents_benchmark_test.go`
and `html/benchmark_test.go`, both guarded by `//go:build go1.24`. Telling
contributors to avoid newer APIs contradicted that, so describe the convention
the repo actually follows.
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (76f04c0) to head (5f2c594).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #340   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          639       639           
=========================================
  Hits           639       639           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

`make benchmark` is a single `go test -bench` invocation, so phrasing it as
comparing before and after implied the target does the comparison itself.
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