Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 18 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand Down
8 changes: 7 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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) ./...

Expand Down
Loading