Document the development workflow in CONTRIBUTING.md - #340
Open
maragubot wants to merge 4 commits into
Open
Conversation
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 Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
`make benchmark` is a single `go test -bench` invocation, so phrasing it as comparing before and after implied the target does the comparison itself.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was found
CONTRIBUTING.mdexplained the social process for contributing — submit a PRdirectly 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
Makefilehastest,lint,cover,benchmark, andfuzztargets, andAGENTS.mddocumentsmake test,make lint, and the coverage rule. ButAGENTS.mdis addressed to AI assistants, so a human contributor had nodocumented 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
## Developmentsection covering the fivemaketargets, the coverageexpectation, and the Go version floor.
Each claim was checked by running it:
make testpasses, and every package outsideinternal/reports 100.0%.The section says "outside
internal/" becausego tool cover -funcreports97.5% overall —
internal/asserthas no tests and is excluded viacodecov.yml, which the docs did not mention anywhere..golangci.ymldeclaresversion: "2", and golangci-lint v1 refuses to runagainst it, so the section asks for v2 specifically.
make fuzzpasses-fuzztime 10sper target, matching a timed run.go 1.18directive stopsnewer 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.goandhtml/benchmark_test.gousetesting.B.Loopbehind//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 coveractually shows, the missing golangci-lint v2requirement, 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.mdnow duplicatesmake 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/appis a separate module with its owngo.modandgo.work, so neithermake testnor CI touches it. A contributor editingthe example app gets no verification at all.