diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index cd437bf..599dd8b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -85,10 +85,15 @@ jobs: cache: false - name: Build for ${{ matrix.goos }}/${{ matrix.goarch }} + # The two roots are the runner and nothing else. An experiment's code is + # in this module and is deliberately outside this pattern, which is + # record 0017 supplying the mechanism record 0009 decided the property + # for. Widening this back to ./... puts a prototype somebody abandoned + # half-written in front of six platforms it was never about. env: GOOS: ${{ matrix.goos }} GOARCH: ${{ matrix.goarch }} - run: go build ./... + run: go build ./cmd/... ./internal/... test: # One entry per platform record 0012 says the release ships a binary for and @@ -166,9 +171,11 @@ jobs: fi - name: Run the default suite - # The same suite on every entry. No platform gets a reduced set and no - # package is excluded here, because the platform whose difficult tests - # are left out is the platform the defect is on. + # The same suite on every entry. No platform gets a reduced set, because + # the platform whose difficult tests are left out is the platform the + # defect is on. The pattern is the runner's two roots on all three, and + # what it leaves out is an experiment's code rather than any of the + # runner's, which is record 0017. # # -count=1 rather than the cache, so a green entry is a suite that ran # on this machine rather than one whose stored output was replayed. @@ -185,7 +192,7 @@ jobs: PLATFORM: ${{ matrix.platform }} run: | set -euo pipefail - go test -count=1 -v ./... 2>&1 | tee suite.log + go test -count=1 -v ./cmd/... ./internal/... 2>&1 | tee suite.log executed=$(grep -cE '^--- (PASS|FAIL): ' suite.log || true) skipped=$(grep -cE '^--- SKIP: ' suite.log || true) echo "${PLATFORM}: executed ${executed} test(s), skipped ${skipped}" @@ -227,7 +234,10 @@ jobs: cache: false - name: Vet the runner - run: go vet ./... + # The step name and the pattern say the same thing now. Record 0017 + # keeps an experiment's code out of every command here, and vet is one + # of them. + run: go vet ./cmd/... ./internal/... format: name: format @@ -252,12 +262,19 @@ jobs: # so the job has to read the output to fail. Printing the names first # means a red run says which files rather than only that some file was # wrong. + # + # gofmt takes directories rather than a package pattern, so the two + # roots are spelled the way it reads them. This is the one command here + # that would fail on an experiment without compiling it: gofmt exits + # non-zero on a file it cannot parse, and half-written is the ordinary + # state of a prototype somebody abandoned. Record 0017 is where that is + # argued. run: | set -euo pipefail - unformatted=$(gofmt -l .) + unformatted=$(gofmt -l cmd internal) if [ -n "$unformatted" ]; then echo "$unformatted" - echo "::error::These files are not gofmt-formatted. Run 'gofmt -w .'." + echo "::error::These files are not gofmt-formatted. Run 'gofmt -w cmd internal'." exit 1 fi - echo "Every Go file in this tree is gofmt-formatted." + echo "Every Go file the runner is built from is gofmt-formatted." diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 094eb2e..16526da 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -71,7 +71,19 @@ jobs: uses: github/codeql-action/init@5595ccaf912efad79be6eef63a5619ff05969be3 # v4 with: languages: go - build-mode: autobuild + # Manual rather than autobuild, because autobuild builds the module + # and the module holds the experiments as well. The subject of this + # workflow is the runner's own source, which is the first line of this + # file, and under autobuild that sentence was not true of what ran. + # Record 0017 is where the pattern below is fixed. + build-mode: manual + + - name: Build the runner + # The same two roots the build workflow, the suite and the guide name. + # A finding is reported against what this step compiled, so widening it + # would put an abandoned prototype in the code-scanning tab under the + # runner's name. + run: go build ./cmd/... ./internal/... - name: Analyse uses: github/codeql-action/analyze@5595ccaf912efad79be6eef63a5619ff05969be3 # v4 diff --git a/.github/workflows/headless.yml b/.github/workflows/headless.yml index 34f2d7b..b1cdec6 100644 --- a/.github/workflows/headless.yml +++ b/.github/workflows/headless.yml @@ -94,14 +94,17 @@ jobs: done - name: Run the default suite - # The same default run, with no opt-in and no reduced set. The count is + # The same default run, with no opt-in and no reduced set. The pattern + # is the runner's two roots, which is what the default run is: record + # 0017 keeps an experiment's code out of every command, and this job + # runs the same string the build workflow and the guide do. The count is # what separates a suite that ran everything and passed from one that # executed nothing and reported success, which look identical from # outside. Top-level results are counted: a subtest's line is indented, # so the anchored pattern does not read it twice. run: | set -euo pipefail - go test -count=1 -v ./... 2>&1 | tee suite.log + go test -count=1 -v ./cmd/... ./internal/... 2>&1 | tee suite.log executed=$(grep -cE '^--- (PASS|FAIL): ' suite.log || true) skipped=$(grep -cE '^--- SKIP: ' suite.log || true) echo "with no display and no elevation: executed ${executed} test(s), skipped ${skipped}" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6c9d07c..bc05eea 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -12,12 +12,22 @@ difference is meant to be visible. Run these, in this order, from the root of a checkout: ``` -go build ./... -go vet ./... -gofmt -l . -go test -count=1 -v ./... +go build ./cmd/... ./internal/... +go vet ./cmd/... ./internal/... +gofmt -l cmd internal +go test -count=1 -v ./cmd/... ./internal/... ``` +Every one of them names the runner rather than the whole module, and an +experiment's code is outside all four. That is +[docs/decisions/0017-what-the-gate-reads-of-an-experiment.md](docs/decisions/0017-what-the-gate-reads-of-an-experiment.md), +which supplies the mechanism record +[docs/decisions/0009-the-checks-do-not-run-experiment-code.md](docs/decisions/0009-the-checks-do-not-run-experiment-code.md) +decided the property for. A prototype you abandon half-written is a thing this +board takes, and it holds nothing red. What follows from that is that nothing +compiles your experiment for you: run it yourself, from a checkout, at the +moment you want the answer. + `gofmt -l` prints the files it would change and exits zero either way, so read its output rather than its exit code. No output is the passing result. diff --git a/docs/decisions/0017-what-the-gate-reads-of-an-experiment.md b/docs/decisions/0017-what-the-gate-reads-of-an-experiment.md new file mode 100644 index 0000000..f041321 --- /dev/null +++ b/docs/decisions/0017-what-the-gate-reads-of-an-experiment.md @@ -0,0 +1,116 @@ +# 0017. What the gate reads of an experiment + +## What was decided + +Every command this repository is gated by names the runner's packages rather +than the whole module. Where a command takes a package pattern that is +`./cmd/... ./internal/...`, and where it takes directories it is `cmd` and +`internal`. It is one set of commands rather than two, so what CONTRIBUTING.md +asks somebody to run before they push and what a job runs are the same strings. + +Record `0009` decided the property this implements. It says nothing that runs on +its own builds or executes anything under `experiments/`, and it names no +mechanism, so the sentence was true of the intention and not of the tree. This +record supplies the mechanism and takes nothing away from that one. + +The state that made the gap visible. An experiment lives inside this module, so +its Go package was in every pattern the commands used, and the first experiment +to land put it there: + + git rev-parse HEAD + 9807dc783b9b981b652bdf00ac1e1467171a1051 + go list ./... + github.com/Flowfin/lab/cmd/lab + github.com/Flowfin/lab/cmd/pullrequest + github.com/Flowfin/lab/experiments/reading-a-tree-of-records + github.com/Flowfin/lab/internal/check + github.com/Flowfin/lab/internal/hardware + github.com/Flowfin/lab/internal/invariants + github.com/Flowfin/lab/internal/prose + github.com/Flowfin/lab/internal/pullrequest + +Six build entries, three suite entries, the vet job and the static analysis all +compiled that package on every pull request, and the formatting job read its +bytes. A prototype that does not compile for one of the six platforms would have +held the board red, and the failure would have named a platform the experiment +was never about. + +The two roots are record `0002`'s list read a second way. That record puts the +entry point in `cmd/lab/` and everything the runner is built from in +`internal/`, and says the runner imports nothing from `experiments/`. So the +question of what the gate covers is a question about the layout rather than +about a pattern somebody picked. + +One thing nothing refuses, written here rather than left to be found. A Go file +at the root of the tree, or under a root directory the layout names that is +neither `cmd/` nor `internal/`, is outside these patterns, and no command +reaches it. A new root directory is refused by +`root-holds-a-directory-the-layout-does-not-name`, so the shape that escapes is +a package added under a directory already named, and what stands behind that is +a reader. That is a smaller hole than the one this record closes and it is not +zero. + +## What it applies to + +Every command in CONTRIBUTING.md and every job that runs one, now and later. A +job added afterwards that walks this module with a Go toolchain takes the same +two patterns rather than the whole module. + +It does not apply to what a person runs in their own checkout. `go run +./experiments/reading-a-tree-of-records` is written in that experiment's record +and still runs from the root of a checkout, which is one of the reasons the +mechanism is this one and not another. + +It does not apply to the checks that read an experiment's record as text. +Record `0009` already separates reading a record from executing an experiment, +and nothing here narrows what the record checks walk. + +## What else was considered + +Giving each experiment its own Go module. + +Holding an experiment's Go files behind a build constraint. + +Holding experiments to the gate and writing that down as deliberate. + +Leaving the patterns alone and letting the first abandoned prototype decide. + +## What each rejected option would have cost + +A module per experiment costs a landed record its command. A directory holding +its own `go.mod` leaves the root module, which is the attraction, because every +pattern stops at it with nothing written down anywhere. The cost arrives at the +other end: `go run ./experiments/reading-a-tree-of-records` stops resolving from +the root of a checkout, and that command is in +`experiments/reading-a-tree-of-records/EXPERIMENT.md` twice, once in the method +and once inside the quoted answer. A record here gains lines and does not have +them replaced, so the repair would be an answered record whose method names a +command that does not run. It also puts a toolchain version in the tree per +experiment, on a board whose whole argument is that writing one should be cheap. + +A build constraint costs the same command for the same reason one level down. A +file every tag excludes is not run by `go run` either, so the record's command +would need a flag it does not carry. It costs one thing the module option does +not: the constraint is a line at the top of a file that somebody has to +remember, nothing refuses its absence, and the day it is forgotten is the day an +abandoned prototype reddens the board, which is the failure being prevented +here. + +Holding experiments to the gate is the option that would replace `0009` rather +than sit beside it, and nothing has happened to weaken the argument that record +makes. Its first reason is still the strongest: the code left behind by an +experiment that answered no is often exactly how it failed, and a gate that goes +red on that code leaves two ways out, deleting the evidence or rewriting the +answer. It would also make every toolchain any experiment ever wants a +dependency of this repository's checks. + +Leaving the patterns alone costs `0009` its meaning. A decision record that is +not true of the tree is worse than an absent one, because it is quoted by +somebody who has no reason to go and check. + +## The condition that reopens this + +Record `0009` carries the condition that reopens the property, and this record +follows it rather than holding one of its own. If experiments start carrying +code that is meant to keep working, the mechanism here is not what is wrong +first.