Repository navigation
Conversation
docker-agent
left a comment
There was a problem hiding this comment.
🟢 No issues found — LGTM! View logs.
glours
left a comment
There was a problem hiding this comment.
I missed this one during my first review
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
AGENTS.md covered build/test/lint mechanics but nothing about how the codebase is organized, so a reader's first mental model comes from file names and local comments — several of which used to be misleading (see the previous commits in this series). The new section states, in ~35 lines, the facts a contributor cannot derive from local reading: the two coexisting lifecycle engines and which commands use which (including that the plan never starts containers), the Run()/EventProcessor operation convention, the stdout/stderr split, the two environment-resolution idioms and the setEnvWithDotEnv timing, the config-hash label as the actual project membership invariant, and the Docker Desktop integration points that must fail silently. Part of docker#14074 (A: the code misdescribes its own structure). Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
d2d17f9 to
db3ff2f
Compare
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
| backend implementation (`NewComposeService`). Everything else under `pkg/` | ||
| is exported for historical reasons, not as a stability promise. | ||
| - **Two lifecycle engines coexist.** The plan-based reconciler | ||
| (`pkg/compose/reconcile.go`) has a single entry point: `create()` — so only |
There was a problem hiding this comment.
[low] Parenthetical points to reconcile.go but create() lives in create.go
The text reads:
The plan-based reconciler (
pkg/compose/reconcile.go) has a single entry point:create()
The parenthetical (pkg/compose/reconcile.go) is most naturally read as the file where create() is defined — but composeService.create() is in pkg/compose/create.go, not in reconcile.go. The package-level reconciler function actually in reconcile.go is named reconcile().
A suggested rewording to avoid the confusion:
| (`pkg/compose/reconcile.go`) has a single entry point: `create()` — so only | |
| - **Two lifecycle engines coexist.** The plan-based reconciler (`pkg/compose/reconcile.go`, called via `composeService.create()` in `create.go`) has a single entry point: `create()` — so only |
| Confidence | Score |
|---|---|
| 🟡 moderate | 67/100 |
AGENTS.md gains a ~35-line architecture map of the facts local reading cannot reveal: the two lifecycle engines and who calls which, the
Run()/events convention distinguishing exported operations from internal helpers, the stdout/stderr split, env-resolution idioms, the config-hash membership invariant, and Docker Desktop integration points.Note: references
service_containers.go, so best merged after #14129.Epic #14074, section A — split out of #14075 for focused review.
🤖 Generated with Claude Code