Skip to content

Commit 41e83fa

Browse files
aledbfclaude
andcommitted
refactor: drop comments that restate the code
Delete about 6,400 comment lines across cmd/ and internal/: doc comments that repeat a name, step narration, banners, "X is TS Y" notes the parity and oracle tests already pin, history of fixed bugs, issue tags, and divergence notes that docs/DIVERGENCES.md or docs/go-only-features.md already record. Kept: comments about behaviour we cannot change (Docker, BuildKit, Compose, git, POSIX shell, Go's standard library, JavaScript and V8 semantics the reference forces), the recipe that regenerates the ts-features-build goldens, and divergences recorded nowhere else. Every //go:, //nolint, mutate-lane and mutate-exempt comment is untouched. Only comments change: each file parses to the same AST once comments are stripped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 5aa1dc3 commit 41e83fa

368 files changed

Lines changed: 489 additions & 6874 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎cmd/devtool/main.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,3 @@
1-
// Command devtool is the repo's CI/dev helper, replacing the former scripts/*.sh.
2-
// Its commands live in internal/devtools (devtools.Main), which wires stdin/stdout,
3-
// git, `go tool`, and docker around the pure, unit-tested logic there. Invoked
4-
// from the Taskfile and the CI workflow (e.g. `go run ./cmd/devtool coverage-gate
5-
// coverage.out TOTAL=48`).
61
package main
72

83
import (

‎cmd/devtool/main_test.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,13 @@ import (
88
"testing"
99
)
1010

11-
// TestMain runs the command itself, instead of the tests, in the child process
12-
// TestTheCommandIsWired starts.
1311
func TestMain(m *testing.M) {
1412
if os.Getenv("DEVTOOL_RUN_MAIN") == "1" {
1513
main()
1614
}
1715
os.Exit(m.Run())
1816
}
1917

20-
// devtool runs this test binary as the devtool command, with args and stdin.
2118
func devtool(t *testing.T, stdin string, args ...string) (code int, stdout, stderr string) {
2219
t.Helper()
2320
cmd := exec.Command(os.Args[0], args...)
@@ -35,8 +32,6 @@ func devtool(t *testing.T, stdin string, args ...string) (code int, stdout, stde
3532
return cmd.ProcessState.ExitCode(), o.String(), e.String()
3633
}
3734

38-
// TestTheCommandIsWired holds what main adds to devtools.Main: the process's
39-
// arguments, standard streams and exit code.
4035
func TestTheCommandIsWired(t *testing.T) {
4136
if code, out, errOut := devtool(t, "internal/cli/exec.go\n", "parity-affected"); code != 0 || out != "exec\n" || errOut != "" {
4237
t.Errorf("parity-affected: code=%d stdout=%q stderr=%q, want 0 \"exec\\n\" \"\"", code, out, errOut)

‎internal/cli/build.go‎

Lines changed: 12 additions & 143 deletions
Large diffs are not rendered by default.

‎internal/cli/build_auth.go‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,7 @@ import (
77
"github.com/devcontainers/cli/internal/oci"
88
)
99

10-
// bridgeBuildAuth resolves credentials for every registry a build touches (base
11-
// image, push tags, cache import/export) via the CLI's credential chain and, when
12-
// any resolve, returns the subprocess env (DOCKER_CONFIG=<tmp>) plus a cleanup
13-
// func. It returns (nil, no-op) when there is nothing to bridge, leaving the
14-
// build's ambient auth untouched. Errors are swallowed (best-effort): a build
15-
// that would have worked anonymously must not fail because auth bridging did.
1610
func bridgeBuildAuth(logger log.Logger, baseImage string, tags, cacheFrom []string, cacheTo string) (env []string, cleanup func()) {
17-
// With no registry to bridge, ResolveBuildAuth resolves nothing and is not ok.
1811
regs := collectBuildRegistries(baseImage, tags, cacheFrom, cacheTo)
1912
dir, clean, ok, err := oci.ResolveBuildAuth(osEnvMap(), regs, logger)
2013
if err != nil || !ok {
@@ -23,7 +16,6 @@ func bridgeBuildAuth(logger log.Logger, baseImage string, tags, cacheFrom []stri
2316
return []string{"DOCKER_CONFIG=" + dir}, clean
2417
}
2518

26-
// collectBuildRegistries returns the deduped registry hosts referenced by a build.
2719
func collectBuildRegistries(baseImage string, tags, cacheFrom []string, cacheTo string) []string {
2820
var out []string
2921
seen := map[string]bool{}
@@ -64,9 +56,6 @@ func registryHostFromImageRef(ref string) string {
6456
return ""
6557
}
6658

67-
// registryCacheRef pulls the image reference out of a buildx cache spec. A
68-
// registry cache is written as "type=registry,ref=<image>[,...]"; anything else
69-
// (type=local/inline/gha, or a bare image) is returned as-is.
7059
func registryCacheRef(spec string) string {
7160
spec = strings.TrimSpace(spec)
7261
if spec == "" || !strings.Contains(spec, "=") {

‎internal/cli/build_auth_held_test.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import (
99
"github.com/devcontainers/cli/internal/log"
1010
)
1111

12-
// isolateBuildAuth points the credential chain at an empty docker config, so
13-
// only DEVCONTAINERS_OCI_AUTH can supply credentials.
1412
func isolateBuildAuth(t *testing.T, ociAuth string) {
1513
t.Helper()
1614
dir := t.TempDir()
@@ -22,8 +20,6 @@ func isolateBuildAuth(t *testing.T, ociAuth string) {
2220
t.Setenv("DEVCONTAINERS_OCI_AUTH", ociAuth)
2321
}
2422

25-
// A build whose registries have no credentials keeps its ambient auth: no
26-
// DOCKER_CONFIG is handed to the build.
2723
func TestBridgeBuildAuthWithoutCredentials(t *testing.T) {
2824
isolateBuildAuth(t, "")
2925
env, cleanup := bridgeBuildAuth(log.Null, "reg.example.com/base:1", nil, nil, "")
@@ -33,7 +29,6 @@ func TestBridgeBuildAuthWithoutCredentials(t *testing.T) {
3329
}
3430
}
3531

36-
// Credentials that resolve are bridged through a temporary DOCKER_CONFIG.
3732
func TestBridgeBuildAuthWithCredentials(t *testing.T) {
3833
isolateBuildAuth(t, "reg.example.com|alice|secret")
3934
env, cleanup := bridgeBuildAuth(log.Null, "reg.example.com/base:1", nil, nil, "")

‎internal/cli/build_auth_test.go‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,11 @@ func TestRegistryHostFromImageRef(t *testing.T) {
1212
"myreg.example.com:5000/team/app": "myreg.example.com:5000",
1313
"localhost:5000/app": "localhost:5000",
1414
"123456.dkr.ecr.us-east-1.amazonaws.com/repo:1": "123456.dkr.ecr.us-east-1.amazonaws.com",
15-
// Docker Hub forms → no explicit registry → "" (ambient auth applies).
16-
"ubuntu:22.04": "",
17-
"library/ubuntu": "",
18-
"org/img:tag": "",
19-
"mcr": "",
20-
"": "",
15+
"ubuntu:22.04": "",
16+
"library/ubuntu": "",
17+
"org/img:tag": "",
18+
"mcr": "",
19+
"": "",
2120
}
2221
for ref, want := range cases {
2322
if got := registryHostFromImageRef(ref); got != want {
@@ -30,9 +29,9 @@ func TestRegistryCacheRef(t *testing.T) {
3029
cases := map[string]string{
3130
"type=registry,ref=ghcr.io/org/cache:main": "ghcr.io/org/cache:main",
3231
"type=registry,ref=myreg.com/c,mode=max": "myreg.com/c",
33-
"type=local,dest=/tmp/c": "", // not a registry cache
32+
"type=local,dest=/tmp/c": "",
3433
"type=inline": "",
35-
"ghcr.io/org/cache:main": "ghcr.io/org/cache:main", // bare ref
34+
"ghcr.io/org/cache:main": "ghcr.io/org/cache:main",
3635
"": "",
3736
}
3837
for spec, want := range cases {
@@ -44,10 +43,10 @@ func TestRegistryCacheRef(t *testing.T) {
4443

4544
func TestCollectBuildRegistries(t *testing.T) {
4645
got := collectBuildRegistries(
47-
"ghcr.io/base/img:1", // base
48-
[]string{"ghcr.io/base/img:1", "reg.example.com/app:2", "ubuntu"}, // tags (dup base, hub)
49-
[]string{"type=registry,ref=cache.example.com/c:main", "type=local,dest=/x"}, // cacheFrom
50-
"type=registry,ref=cache.example.com/c:main", // cacheTo (dup of cacheFrom)
46+
"ghcr.io/base/img:1",
47+
[]string{"ghcr.io/base/img:1", "reg.example.com/app:2", "ubuntu"},
48+
[]string{"type=registry,ref=cache.example.com/c:main", "type=local,dest=/x"},
49+
"type=registry,ref=cache.example.com/c:main",
5150
)
5251
want := []string{"ghcr.io", "reg.example.com", "cache.example.com"}
5352
if !reflect.DeepEqual(got, want) {
@@ -56,7 +55,6 @@ func TestCollectBuildRegistries(t *testing.T) {
5655
}
5756

5857
func TestCollectBuildRegistriesEmpty(t *testing.T) {
59-
// Only Docker Hub / local caches → nothing to bridge.
6058
if got := collectBuildRegistries("ubuntu:22.04", []string{"org/app"}, []string{"type=inline"}, ""); len(got) != 0 {
6159
t.Fatalf("expected no registries, got %v", got)
6260
}

‎internal/cli/build_cachefrom_test.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,6 @@ import (
77
"github.com/devcontainers/cli/internal/config"
88
)
99

10-
// TestCacheFromForDockerfileBuild pins the merge of the --cache-from flag with
11-
// devcontainer.json build.cacheFrom: flag values first, then config values, in
12-
// order — matching TS singleContainer (additionalCacheFroms, then config.build.cacheFrom).
1310
func TestCacheFromForDockerfileBuild(t *testing.T) {
1411
cfgWith := func(cf ...string) *config.DevContainer {
1512
return &config.DevContainer{Build: &config.Build{CacheFrom: config.StringOrStrings(cf)}}
@@ -38,9 +35,6 @@ func TestCacheFromForDockerfileBuild(t *testing.T) {
3835
}
3936
}
4037

41-
// TestCacheFromForDockerfileBuildDoesNotMutateFlag guards against the helper
42-
// aliasing/appending into the caller's flag slice (which extendImageWithFeatures
43-
// still passes verbatim for the feature layers).
4438
func TestCacheFromForDockerfileBuildDoesNotMutateFlag(t *testing.T) {
4539
flagBacking := []string{"flag1", "sentinel"}
4640
flag := flagBacking[:1]

‎internal/cli/build_compose_path_test.go‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,6 @@ import (
99
"github.com/devcontainers/cli/internal/log"
1010
)
1111

12-
// TestBuildComposePathWiring verifies that the `build` command threads
13-
// --docker-compose-path into docker.NewComposeClient. When the call site
14-
// hardcodes "" instead, the operator-supplied path could never appear in the
15-
// compose-client detection error ("neither 'docker compose' nor '<path>'
16-
// found"). The test is hermetic: dockerPath points at a nonexistent binary so
17-
// the `docker compose` (v2) probe always fails regardless of the host, forcing
18-
// NewComposeClient down the custom-path branch.
1912
func TestBuildComposePathWiring(t *testing.T) {
2013
tests := []struct {
2114
name string
@@ -39,8 +32,6 @@ func TestBuildComposePathWiring(t *testing.T) {
3932
r := &buildRunner{
4033
log: log.New(log.Options{Writer: io.Discard}),
4134
opts: &buildOpts{
42-
// Force the `docker compose` (v2) probe to fail hermetically,
43-
// independent of whether Docker is installed on the host.
4435
dockerPath: "/nonexistent/docker-binary",
4536
dockerComposePath: tt.dockerComposePath,
4637
},

‎internal/cli/build_held_test.go‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ import (
1717

1818
func discardLogger() log.Logger { return log.New(log.Options{Writer: io.Discard}) }
1919

20-
// The deprecated --experimental-frozen-lockfile and --frozen-lockfile both
21-
// make the Feature build verify the lockfile, as TS effectiveFrozenLockfile.
2220
func TestBuildFrozenLockfileReachesTheFeatureBuild(t *testing.T) {
2321
for _, tc := range []struct {
2422
name string
@@ -187,9 +185,6 @@ func TestBuildxSecrets(t *testing.T) {
187185
}
188186
}
189187

190-
// --buildkit never skips BuildKit detection, so the build runs as if buildx
191-
// were absent even when the Docker CLI has it. The fake --docker-path reports
192-
// buildx, and both cases stop at the flag check before any Docker build.
193188
func TestRunBuildBuildKitNeverIgnoresBuildx(t *testing.T) {
194189
bin := t.TempDir()
195190
docker := filepath.Join(bin, "docker")
@@ -227,9 +222,6 @@ func TestRunBuildBuildKitNeverIgnoresBuildx(t *testing.T) {
227222
}
228223
}
229224

230-
// The Dockerfile build targets the config's target when it names one, else
231-
// the final stage, named when it has no name; a Dockerfile without FROM is
232-
// TS's error.
233225
func TestPrepareDockerfileBuildStage(t *testing.T) {
234226
ws := writeConfigFixture(t, map[string]string{
235227
".devcontainer/Dockerfile": "FROM alpine:3 AS base\nFROM base\n",
@@ -257,9 +249,6 @@ func TestPrepareDockerfileBuildStage(t *testing.T) {
257249
}
258250
}
259251

260-
// A Compose build without --image-name reports its image as a string, as TS
261-
// doBuild does (imageNameResult = originalImageName); with --image-name, and
262-
// for any other configuration, the names are an array.
263252
func TestBuildResultImageName(t *testing.T) {
264253
for _, tc := range []struct {
265254
name string

‎internal/cli/build_nodocker_held_test.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,6 @@ import (
77
"testing"
88
)
99

10-
// A build whose image or Dockerfile step fails before any Docker call reports
11-
// the failure on stdout and exits 1, never the success envelope: a legacy
12-
// Feature id as the bare message, any other failure as a container error.
1310
func TestRunBuildReportsAFailedBuildStep(t *testing.T) {
1411
for name, tc := range map[string]struct {
1512
config string

0 commit comments

Comments
 (0)