This is the goimports half of #119. The Markdown half is a question for sneak in the comment below, and the issue stays open for it.
script/fmt-check runs goimports -l and fails naming any file it would change, the way it already did for gofmt. It now also checks gofmt -s, which script/fmt applies but the check left out.
script/fmt and script/fmt-check run goimports with go run at the commit script/bootstrap used to pin. A goimports on PATH is never used, and script/bootstrap no longer installs it.
Not visible in the diff:
The pin is written in both scripts, and each says it must match the other.
go run needs the network the first time it runs on a machine. The Dockerfile lint stage runs make fmt-check in the golangci-lint image, so it downloads and builds goimports on every build.
How sneak's other Go repos run prettier: webhooker and pixa do not run it. cattbox adds node to its golangci-lint lint stage, and template-app-go (also hostsurvey, homoicon) runs it in a separate build on a node image, outside the lint stage. None runs it in the lint stage without node, so following the plan, this PR stops there.
Disclosures:
Deviation: the commit title ends (#119), not (closes #119), because the Markdown half is still open.
Judgement call: -s added to the gofmt check, under the issue's "verifies everything script/fmt applies".
Model: opus-5-5
This is the `goimports` half of https://git.eeqj.de/sneak/dnswatcher/issues/119. The Markdown half is a question for sneak in the comment below, and the issue stays open for it.
- `script/fmt-check` runs `goimports -l` and fails naming any file it would change, the way it already did for `gofmt`. It now also checks `gofmt -s`, which `script/fmt` applies but the check left out.
- `script/fmt` and `script/fmt-check` run `goimports` with `go run` at the commit `script/bootstrap` used to pin. A `goimports` on `PATH` is never used, and `script/bootstrap` no longer installs it.
Not visible in the diff:
- The pin is written in both scripts, and each says it must match the other.
- `go run` needs the network the first time it runs on a machine. The Dockerfile lint stage runs `make fmt-check` in the golangci-lint image, so it downloads and builds `goimports` on every build.
How sneak's other Go repos run prettier: `webhooker` and `pixa` do not run it. `cattbox` adds node to its golangci-lint lint stage, and `template-app-go` (also `hostsurvey`, `homoicon`) runs it in a separate build on a node image, outside the lint stage. None runs it in the lint stage without node, so following the plan, this PR stops there.
Disclosures:
- Deviation: the commit title ends `(#119)`, not `(closes #119)`, because the Markdown half is still open.
- Judgement call: `-s` added to the `gofmt` check, under the issue's "verifies everything `script/fmt` applies".
Model: opus-5-5
script/fmt-check now runs goimports in list mode and fails naming any
file it would change, and checks gofmt with -s, as script/fmt applies
it. Both scripts run goimports with `go run` at the commit that
script/bootstrap used to install, so a goimports on PATH is never used
and bootstrap no longer installs it. The pin is written in both
scripts; change them together. The first run on a machine, and every
Dockerfile lint stage run, downloads and builds goimports.
The Markdown half of the issue (prettier) is not done here: it needs
node in the lint image or a separate build, a decision for the owner.
Model: opus-5-5
@sneak The Markdown half of #119 needs your decision, and this PR does not do it.
prettier runs on node. The Dockerfile lint stage runs make fmt-check in the golangci-lint image, which has no node. Your other Go repos do it one of two ways:
cattbox: script/bootstrap installs a pinned node with nvm, then yarn and prettier from yarn.lock, and its lint stage runs that bootstrap. This puts node in the lint image, and make fmt-check stays one command everywhere.
template-app-go (also hostsurvey, homoicon): a Dockerfile.check stage on a digest-pinned node image installs prettier from package.json and its lockfile. make fmt and make fmt-check run it through docker build, and script/cibuild runs that check next to the main build. The Dockerfile lint stage then checks only the Go. This puts no node on the host or in the lint image.
Either way, *.md has to come out of .dockerignore, because today no Markdown reaches a Docker build.
My recommendation is 2, using yarn.lock instead of package-lock.json, because REPO_POLICIES.md says to use yarn. Which way do you want it done?
Model: opus-5-5
@sneak The Markdown half of https://git.eeqj.de/sneak/dnswatcher/issues/119 needs your decision, and this PR does not do it.
prettier runs on node. The Dockerfile lint stage runs `make fmt-check` in the golangci-lint image, which has no node. Your other Go repos do it one of two ways:
1. `cattbox`: `script/bootstrap` installs a pinned node with nvm, then yarn and prettier from `yarn.lock`, and its lint stage runs that bootstrap. This puts node in the lint image, and `make fmt-check` stays one command everywhere.
2. `template-app-go` (also `hostsurvey`, `homoicon`): a `Dockerfile.check` stage on a digest-pinned node image installs prettier from `package.json` and its lockfile. `make fmt` and `make fmt-check` run it through `docker build`, and `script/cibuild` runs that check next to the main build. The Dockerfile lint stage then checks only the Go. This puts no node on the host or in the lint image.
Either way, `*.md` has to come out of `.dockerignore`, because today no Markdown reaches a Docker build.
My recommendation is 2, using `yarn.lock` instead of `package-lock.json`, because `REPO_POLICIES.md` says to use yarn. Which way do you want it done?
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
This is the
goimportshalf of #119. The Markdown half is a question for sneak in the comment below, and the issue stays open for it.script/fmt-checkrunsgoimports -land fails naming any file it would change, the way it already did forgofmt. It now also checksgofmt -s, whichscript/fmtapplies but the check left out.script/fmtandscript/fmt-checkrungoimportswithgo runat the commitscript/bootstrapused to pin. AgoimportsonPATHis never used, andscript/bootstrapno longer installs it.Not visible in the diff:
go runneeds the network the first time it runs on a machine. The Dockerfile lint stage runsmake fmt-checkin the golangci-lint image, so it downloads and buildsgoimportson every build.How sneak's other Go repos run prettier:
webhookerandpixado not run it.cattboxadds node to its golangci-lint lint stage, andtemplate-app-go(alsohostsurvey,homoicon) runs it in a separate build on a node image, outside the lint stage. None runs it in the lint stage without node, so following the plan, this PR stops there.Disclosures:
(#119), not(closes #119), because the Markdown half is still open.-sadded to thegofmtcheck, under the issue's "verifies everythingscript/fmtapplies".Model: opus-5-5
@sneak The Markdown half of #119 needs your decision, and this PR does not do it.
prettier runs on node. The Dockerfile lint stage runs
make fmt-checkin the golangci-lint image, which has no node. Your other Go repos do it one of two ways:cattbox:script/bootstrapinstalls a pinned node with nvm, then yarn and prettier fromyarn.lock, and its lint stage runs that bootstrap. This puts node in the lint image, andmake fmt-checkstays one command everywhere.template-app-go(alsohostsurvey,homoicon): aDockerfile.checkstage on a digest-pinned node image installs prettier frompackage.jsonand its lockfile.make fmtandmake fmt-checkrun it throughdocker build, andscript/cibuildruns that check next to the main build. The Dockerfile lint stage then checks only the Go. This puts no node on the host or in the lint image.Either way,
*.mdhas to come out of.dockerignore, because today no Markdown reaches a Docker build.My recommendation is 2, using
yarn.lockinstead ofpackage-lock.json, becauseREPO_POLICIES.mdsays to use yarn. Which way do you want it done?Model: opus-5-5
Review passed on
c498246.Model: opus-5-5