diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 81ebc67..611a840 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -6,7 +6,7 @@ - [ ] The title follows [Conventional Commits](https://www.conventionalcommits.org/) (`feat:`, `fix:`, `docs:`, `chore:`...) -- [ ] Tests are added or updated, and `make test` passes +- [ ] Tests are added or updated, and `make check` passes - [ ] Documentation is up to date, `README.md` and `README.fr.md` alike - [ ] A user-visible change has an entry under `[Unreleased]` in `CHANGELOG.md` - [ ] No secret, credential or real account data appears anywhere in the change diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 937506c..b3f149b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,6 +28,8 @@ without network access; a test that needs a credential is a bug in the test. | `make test` | Run the tests with the race detector and coverage | | `make lint` | Check `gofmt`, `go mod tidy` and run golangci-lint | | `make vuln` | Run govulncheck | +| `make markdown` | Check the Markdown files against `.markdownlint-cli2.jsonc` (needs `npx`) | +| `make check` | Run every check CI runs, in its order: lint, Markdown, tests, govulncheck | | `make fuzz` | Explore every fuzz target, 30 seconds each, or `FUZZTIME` | | `make snapshot` | Regenerate the embedded API catalogue | | `make demo` | Run the interface on invented data, without an OVHcloud account | @@ -58,8 +60,9 @@ invented keys. 4. Open a pull request against `main` and fill in the checklist. The pull request title follows Conventional Commits too, because pull requests are squash-merged and the title becomes the commit message. -5. CI must pass: formatting, `go mod tidy`, golangci-lint, tests, govulncheck, CodeQL and the - multi-architecture image build. +5. CI must pass: formatting, `go mod tidy`, golangci-lint, Markdown, tests, govulncheck, CodeQL + and the multi-architecture image build. `make check` runs the same checks locally, but for + CodeQL and the image, before you open the pull request. ## What every change needs @@ -112,10 +115,19 @@ binary, with no build step. Alpine.js is vendored in its CSP build, which evalua expression strings: a directive may only name a property or a method, and anything computed belongs in `app.js`. This is what keeps the content security policy free of `unsafe-eval`. -Since nothing compiles these files, `go test ./internal/web` checks them: duplicated or unused -CSS classes, undeclared custom properties, directives holding expressions, calls to undefined -methods, unused members, error codes the backend does not send, and any resource loaded from -outside the binary. +Since nothing compiles these files, `go test ./internal/web` checks them, and refuses things no +other Go project refuses. Each of these mistakes has shipped once and failed silently in a +browser, which is why they fail loudly here. When one of them stops you: + +| A failure about | What it means, and the usual fix | +| --- | --- | +| a directive holding an expression | Alpine's CSP build cannot evaluate it. Move the logic into a getter or a method of `app.js`, and name that in the markup. | +| a label defined and asked for nowhere, or asked for and defined nowhere | Every label exists in both `en` and `fr`, and is shown somewhere. Add the missing translation, or remove a label no screen uses any more. | +| a CSS class unused, unstyled, defined twice, or used as a modifier and as a component | Remove the leftover rule or class, or rename one of the two so they do not collide. | +| a custom property read but not declared, or declared but not read | Declare it in the `:root` blocks of `app.css`, or remove it. | +| a method called but not defined, or a member or top-level declaration nothing reads | Define the method, or remove what a removed screen left behind. | +| an error code or a finding code | The codes the backend sends and the audit raises must be exactly those the dictionaries word, in both languages. Add the wording, or remove a code that is no longer sent. | +| a resource loaded from outside the binary | No CDN, font, script or image from elsewhere: vendor it under `static/`, or use a data URI. | A new audit finding needs a key in `tools/demo` that raises it: `go test ./tools/demo` fails until there is one, so that every finding can be seen before it ships. diff --git a/Makefile b/Makefile index 8d3ac57..551680c 100644 --- a/Makefile +++ b/Makefile @@ -30,11 +30,13 @@ TOOLCHAIN := $(shell awk '/^toolchain/ {print $$2}' go.mod) .DEFAULT_GOAL := help -.PHONY: help build test lint vuln fuzz snapshot demo docker +.PHONY: help check build test lint markdown vuln fuzz snapshot demo docker help: ## List the available targets @awk 'BEGIN {FS = ":.*## "} /^[a-z-]+:.*## / {printf " %-10s %s\n", $$1, $$2}' $(MAKEFILE_LIST) +check: lint markdown test vuln ## Run every check CI runs, in its order: what a pull request is held to + build: ## Build the keymaker binary into ./bin CGO_ENABLED=0 go build -trimpath -ldflags "$(LDFLAGS)" -o bin/keymaker ./cmd/keymaker @@ -46,6 +48,10 @@ lint: ## Check formatting, module tidiness and run golangci-lint go mod tidy -diff golangci-lint run ./... +markdown: ## Check the Markdown files against .markdownlint-cli2.jsonc, as CI does (needs npx) + @command -v npx >/dev/null || { echo "npx not found: install Node.js to check the Markdown as CI does"; exit 1; } + npx --yes markdownlint-cli2 $$(git ls-files '*.md') + vuln: ## Check dependencies and the standard library against the Go vulnerability database GOTOOLCHAIN=$(TOOLCHAIN) go run golang.org/x/vuln/cmd/govulncheck@$(GOVULNCHECK_VERSION) ./...